[PATCH] D135574: [MemCpyOpt] Update code comments for processMemSetMemCpyDependence. NFC
Bjorn Pettersson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 10 06:23:44 PDT 2022
bjope added inline comments.
================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1279-1282
Value *Ule = Builder.CreateICmpULE(DestSize, SrcSize);
Value *SizeDiff = Builder.CreateSub(DestSize, SrcSize);
Value *MemsetLen = Builder.CreateSelect(
Ule, ConstantInt::getNullValue(DestSize->getType()), SizeDiff);
----------------
Side note: This can be replaced by an usub_sat. I guess it would be safe to do that directly here instead of letting InstCombine detect the pattern later?
================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:1293
"MemCpy must be a MemoryDef");
- // The new memset is inserted after the memcpy, but it is known that its
- // defining access is the memset about to be removed which immediately
- // precedes the memcpy.
+ // The new memset is inserted before the memcpy, and it is known that the
+ // memcpy:s defining access is the memset about to be removed.
----------------
@fhahn : I think you added the MSSA updating here. But the code comments did not match up with the actual movement of the memset. But I think the actual MSSA update seem to match the implementation, so it was only code comment that was wrong. Right?
And as I hinted elsewhere, maybe we want this transform to move the memset below the memcpy. But that seem to make the MSSA update a bit trickier. I kind of assumed that one could use MSSAU->moveAfter/moveBefore in a situation like this? But maybe that require that the memset instruction only is moved and not replaced (as we need to adjust the arguments here) in some way?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135574/new/
https://reviews.llvm.org/D135574
More information about the llvm-commits
mailing list