zaks.anna added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp:95
@@ +94,3 @@
+  if (Optional<PreStmt> SP = N->getLocation().getAs<PreStmt>()) {
+    if (const CallExpr *CE = clang::dyn_cast<CallExpr>(SP->getStmt())) {
+
----------------
Would it be possible to identify the "interesting node" by just looking at how 
the state you track changes. This function will be called when we walk the path 
upward from the error node. For example, you can notice that in the previous 
state, the Request did not exist and in this state it is Nonblocking. That 
would allow you to conclude that between those 2 states a function was called. 
(This is the trick we use in the other checkers (ex: malloc); it simplifies 
implementation and makes it more robust - we do not need to pattern match as 
much.)

================
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:86
@@ +85,3 @@
+      // A wait has no matching nonblocking call.
+      BReporter->reportUnmatchedWait(PreCallEvent, ReqRegion, ErrorNode);
+    }
----------------
Can you add a test cases for this report when there are multiple ReqRegions? 
Looks like all of the test cases with MPI_Waitall succeed. (I test a case where 
one of the regions has a match in Waitall and one does not as well as when 
several/all do not have a match.)

================
Comment at: test/Analysis/MPIChecker.cpp:113
@@ +112,3 @@
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, 
&rs.req); // expected-note{{Request is previously used by nonblocking call 
here.}}
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, 
&rs.req); // expected-warning{{Double nonblocking on request 'rs.req'.}}
+  MPI_Wait(&rs.req, MPI_STATUS_IGNORE);
----------------
Are we allowed to call reduce -> wait -> reduce with the same req? If so, let's 
test for that.

Also, can calls to reduce and wait occur in different functions? If so, we need 
to test cases where they occur in different functions that are all in the same 
translation unit - all of the implementations are visible to the analyzer. And 
also test the cases where we pass the request to a function, whose body is not 
visible to the analyzer, which is the case when the function is defined in 
another translation unit.



http://reviews.llvm.org/D12761



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to