[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-09-06 Thread Donát Nagy via Phabricator via cfe-commits
donat.nagy requested changes to this revision. donat.nagy added a comment. This revision now requires changes to proceed. I reviewed this change and collected my suggestions in comments. In general, I feel that the code added by this commit is "sloppy" in the sense that it's designed for the com

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-09-05 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. I don't have time this week, sorry. Maybe others can take this over. @donat.nagy could you please have a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158813/new/ https://reviews.llvm.org/D158813 ___

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-09-05 Thread Ding Fei via Phabricator via cfe-commits
danix800 added a comment. Ping~, could anyone have a look at this revision please? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158813/new/ https://reviews.llvm.org/D158813 ___ cfe-commits maili

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-08-25 Thread Ding Fei via Phabricator via cfe-commits
danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:195 + const NonLoc Idx = Ctx.getSValBuilder().makeArrayIndex(i); + auto CountReached = SVB.evalBinOp(State, BO_GE, Idx, Count, ASTCtx.BoolTy) +

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-08-25 Thread Ding Fei via Phabricator via cfe-commits
danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:192-198 +SVal Count = CE.getArgSVal(0); +for (size_t i = 0; i < ArrSize; ++i) { + const NonLoc Idx = Ctx.getSValBuilder().makeArrayIndex(i); + auto CountReac

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-08-25 Thread Ding Fei via Phabricator via cfe-commits
danix800 added inline comments. Comment at: clang/test/Analysis/mpichecker.cpp:286-288 + for (int i = 0; i < 3; ++i) +MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, +&rs.req[i]); steakhal wrote: > This loop here im

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-08-25 Thread Ding Fei via Phabricator via cfe-commits
danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:189-190 +ASTCtx.getCharWidth(); +const NonLoc MROffset = +SVB.makeArrayIndex(MR->getAsOffset().getOffset() / ElemSizeInBits); steakhal w

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-08-25 Thread Ding Fei via Phabricator via cfe-commits
danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:185-188 +CharUnits ElemSizeInChars = ASTCtx.getTypeSizeInChars(ElemType); +int64_t ElemSizeInBits = +(ElemSizeInChars.isZero() ? 1 : ElemSizeInChars.getQuantit

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-08-25 Thread Ding Fei via Phabricator via cfe-commits
danix800 added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:46-48 +if (!ErrorNode) + return; + steakhal wrote: > If these hunks are not closely related to the issue you intend to fix in this > PR, I'd suggess

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-08-25 Thread Ding Fei via Phabricator via cfe-commits
danix800 added a comment. In D158813#4616816 , @steakhal wrote: > Let me try to summarize some of the variables: > > So, given an invocation like `MPI_Waitall(C, Requests, Statues)`: > > - `MR` is `lval(Requests)` > - `ElementCount` is the number of eleme

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-08-25 Thread Ding Fei via Phabricator via cfe-commits
danix800 updated this revision to Diff 553514. danix800 edited the summary of this revision. danix800 added a comment. 1. Move out complicated computation into separate function; 2. Only check `ConreteInt` request count. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-08-25 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment. Let me try to summarize some of the variables: So, given an invocation like `MPI_Waitall(C, Requests, Statues)`: - `MR` is `lval(Requests)` - `ElementCount` is the number of elements `Requests` consist of. - `ArrSize` is basically `ElementCount` but an APSInt. - `MROffs

[PATCH] D158813: [analyzer] MPIChecker: MPI_Waitall should respect count arg

2023-08-24 Thread Ding Fei via Phabricator via cfe-commits
danix800 created this revision. danix800 added reviewers: steakhal, donat.nagy, dcoughlin. danix800 added a project: clang. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. H