[PATCH] D108289: [NFC][LoopIdiom] Let processLoopStoreOfLoopLoad take StoreSize as SCEV instead of unsigned
Bardia Mahjour via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 18 11:17:07 PDT 2021
bmahjour added inline comments.
================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:891-892
- return processLoopStoreOfLoopLoad(Dest, Source, (unsigned)SizeInBytes,
+ const SCEV *MemCpySizeSCEV = SE->getSCEV(MCI->getLength());
+ return processLoopStoreOfLoopLoad(Dest, Source, MemCpySizeSCEV,
MCI->getDestAlign(), MCI->getSourceAlign(),
----------------
why not:
```
return processLoopStoreOfLoopLoad(Dest, Source, SE->getSCEV(SizeInBytes),
```
?
================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1247
+
+ const SCEV *StoreSizeSCEV = SE->getConstant(StorePtr->getType(), StoreSize);
+ return processLoopStoreOfLoopLoad(StorePtr, LoadPtr, StoreSizeSCEV,
----------------
StorePtr->getType() is the type of the pointer, did you mean `SI->getValueOperand()->getType()`?
================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1281
+ const SCEVConstant *ConstStoreSize = dyn_cast<SCEVConstant>(StoreSizeSCEV);
+ int64_t StoreSize = ConstStoreSize->getValue()->getSExtValue();
bool IsNegStride = StoreSize == -Stride;
----------------
this is being sign extended but SCEV being passed to `processLoopStoreOfLoopLoad` is created using `ScalarEvolution::getConstant` with the default value of `false` for the `isSigned` parameter.
================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1281
+ const SCEVConstant *ConstStoreSize = dyn_cast<SCEVConstant>(StoreSizeSCEV);
+ int64_t StoreSize = ConstStoreSize->getValue()->getSExtValue();
bool IsNegStride = StoreSize == -Stride;
----------------
bmahjour wrote:
> this is being sign extended but SCEV being passed to `processLoopStoreOfLoopLoad` is created using `ScalarEvolution::getConstant` with the default value of `false` for the `isSigned` parameter.
```
/// TODO: until non-constant sizes are handled
assert(ConstStoreSize && "store size is expected to be a constant");
```
or add logic to deal with non-constant situations.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108289/new/
https://reviews.llvm.org/D108289
More information about the llvm-commits
mailing list