[PATCH] D155520: [LV] Complete load groups and release store groups in presence of dependency
Ayal Zaks via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Jul 30 07:53:55 PDT 2023
Ayal added a comment.
In D155520#4539500 <https://reviews.llvm.org/D155520#4539500>, @anna wrote:
> fixed use-after-free error (with added testcase). This bug wasn't there before the patch because we would break out of the inner loop accessing A, whereas now we were continuing to see which other A accesses needed to release the group.
Hmm, this error seems to stem from a case of "Too many dependences, stopped recording" (LoopAccessAnalysis.cpp), which leads `canReorderMemAccessesForInterleavedGroups()` to answer false for any given pair of stores - even those joined together in the same interleave group. However, all members of an interleave group are effectively checked for independence upon insertion, implying they are pairwise reorderable, even in the absence of recorded dependencies.
An alternative outlined above is to augment `canReorderMemAccessesForInterleavedGroups()` with a pre-check if the two instructions belong to the same (store) group.
Having `Dependences.size()` surpass `MaxDependences` calls for an extensive testcase, given the latter defaults to 100.
An alternative is to reduce this threshold by setting `max-dependences` thereby helping to produce a simpler minimal testcase.
================
Comment at: llvm/lib/Analysis/VectorUtils.cpp:1203
- // If a dependence exists and A is not already in a group (or it was
- // and we just released it), B might be hoisted above A (if B is a
- // load) or another store might be sunk below A (if B is a store). In
- // either case, we can't add additional instructions to B's group. B
- // will only form a group with instructions that it precedes.
- break;
+ if (A->mayWriteToMemory()) { // Otherwise dependencies are tolerable.
+ Instruction *DependentInst = nullptr;
----------------
Need to also move the definition of GroupA earlier.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155520/new/
https://reviews.llvm.org/D155520
More information about the llvm-commits
mailing list