[PATCH] D129958: [IndVars] Eliminate redundant type cast with different sizes

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyIndVar.cpp:696
   if (MaskBits <= DestNumSigBits) {
-    for (User *U : UseInst->users()) {
+    for (User *U : make_early_inc_range(UseInst->users())) {
       // Match for fptosi/fptoui of sitofp and with same type.
----------------
Why do we need an early inc range now? We're adding a new user of the IVOperand, not the UseInst, right?


================
Comment at: llvm/lib/Transforms/Utils/SimplifyIndVar.cpp:706
 
-      CI->replaceAllUsesWith(IVOperand);
+      PHINode *Phi = cast<PHINode>(IVOperand);
+      Value *Conv = nullptr;
----------------
I don't think the IVOperand is necessarily a phi node. Can you add a test where the itofp is on the postinc IV (i.e. the add)?


================
Comment at: llvm/test/Transforms/IndVarSimplify/floating-point-small-iv.ll:138
+  %idxprom32 = sext i16 %conv to i32
+  %idxprom64 = sext i16 %conv to i64
+  %arrayidx = getelementptr inbounds [16777219 x i32], [16777219 x i32]* @array, i64 0, i64 %idxprom64
----------------
I don't understand these tests. We don't want an existing sext user, but rather a wider type on the fptosi. So sitofp i16 and then fptosi i32.


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

https://reviews.llvm.org/D129958