[PATCH] D104595: [NFC] [LoopIdiom] Let processLoopStridedStore take StoreSize as SCEV instead of unsigned

Yueh-Ting Chen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 4 12:05:28 PDT 2021


eopXD added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:916
 
+  const SCEV *MemsetSizeSCEV = SE->getSCEV(MSI->getLength());
+  if (!MemsetSizeSCEV)
----------------
Whitney wrote:
> Is it better to have line 916-918 after 921-923?
> 
> In what case, `!MemsetSizeSCEV`? Do we early exit more often than before?
The variable is not used other than forwarding the store size of the memset as SCEV into `processLoopStridedStore`.  So I deleted this variable. Also the if-statement will never be triggered and is misleading.



================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1151
 
-  const SCEV *NumBytesS =
-      getNumBytes(BECount, IntIdxTy, StoreSize, CurLoop, DL, SE);
+  // NumBytes = TripCount * StoreSize
+  const SCEV *TripCountS = getTripCount(BECount, IntIdxTy, CurLoop, DL, SE);
----------------
Whitney wrote:
> Is this really NFC?
Right here the code does the exact same thing as above except no conversion on StoreSize is needed. (since its now a SCEV).

Created another `getNumBytes` function that takes `StoreSize` as SCEV, which better shows that this is a NFC.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104595/new/

https://reviews.llvm.org/D104595



More information about the llvm-commits mailing list