[PATCH] D75980: [LV] Generate RT checks up-front and remove them if required.
Evgeniy via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 24 01:53:40 PST 2021
ebrevnov added a comment.
Thanks for doing the changes. Overall this version looks fine to me. Please go ahead with some final cleanups.
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:748
/// Emit bypass checks to check any memory assumptions we may have made.
- void emitMemRuntimeChecks(Loop *L, BasicBlock *Bypass);
+ BasicBlock *emitMemRuntimeChecks(Loop *L, BasicBlock *Bypass);
----------------
Please update this one as well.
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1996-1997
+ // Memory runtime check generation creates compares that use expanded
+ // values. Remove them before running the clerunning the clerunning the
+ // cleaner.
+ for (auto &I : make_early_inc_range(reverse(*MemCheckBlock))) {
----------------
Please revisit
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2042
+ BranchInst::Create(Bypass, LoopVectorPreHeader, SCEVCheck));
+ SCEVCheck = nullptr;
+ return SCEVCheckBlock;
----------------
Why do we need to set it to null?
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2069
+
+ auto *C = MemCheckBlock;
+ MemRuntimeCheck = nullptr;
----------------
Why storing to temp?
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2070
+ auto *C = MemCheckBlock;
+ MemRuntimeCheck = nullptr;
+ return C;
----------------
Why needed?
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1915
+ BasicBlock *LoopHeader = Preheader->getSingleSuccessor();
+ TmpBlock = SplitBlock(Preheader, Preheader->getTerminator(), DT, LI,
+ nullptr, "tmp.rtchecks");
----------------
fhahn wrote:
> ebrevnov wrote:
> > The goal is to create "un-linked" block(s). Right? In that case, would it be simpler to not depend on Preheader and use CreateBlock directly. I believe it would be easier to un-link the block then.
> I updated the code put SCEV and memory checks in separate blocks. It still uses `SplitBlock`, because this keeps the blocks in the DT/LI, which may be used during SCEV expansion and it also keeps them linked to the right predecessors during expansion (SCEVExpander tries to find values to re-use in predecessors).
Please, put that info into the code as a comment. SCEVExpander also can decide to hoist some instruction(s) into predecessors. Will they be properly scheduled for deletion?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75980/new/
https://reviews.llvm.org/D75980
More information about the llvm-commits
mailing list