[PATCH] D129140: [IndVars] Directly use integer induction for FPToSI of float induction.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 442957.
fhahn marked an inline comment as done.
fhahn added a comment.

Add fptosi to DeadInsts as suggested, thanks!

In D129140#3634418 <https://reviews.llvm.org/D129140#3634418>, @Allen wrote:

> In D129140#3630425 <https://reviews.llvm.org/D129140#3630425>, @reames wrote:
>
>> No objection here, but I want to note that recomputing the range should be as simple as asking SCEV for the range of the new IV.  We'd basically just need a match for fptosi of sitofp, and then query the range of the input to the sitofp.  We should probably be doing that in addition to this patch.
>>
>> Edit: getSignedRange after calling computeSCEVAtScope that is.
>
> hi @reames , out of interest , I tried to recomputing the range in https://reviews.llvm.org/D129191 as an addition patch.

Thanks for sharing the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129140

Files:
  llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
  llvm/test/Transforms/IndVarSimplify/floating-point-iv.ll


Index: llvm/test/Transforms/IndVarSimplify/floating-point-iv.ll
===================================================================
--- llvm/test/Transforms/IndVarSimplify/floating-point-iv.ll
+++ llvm/test/Transforms/IndVarSimplify/floating-point-iv.ll
@@ -382,8 +382,7 @@
 ; CHECK-NEXT:    [[FLOAT_IV_INT:%.*]] = phi i32 [ 1000, [[ENTRY:%.*]] ], [ [[FLOAT_IV_NEXT_INT:%.*]], [[LOOP]] ]
 ; CHECK-NEXT:    [[INDVAR_CONV:%.*]] = sitofp i32 [[FLOAT_IV_INT]] to float
 ; CHECK-NEXT:    call void @use.float(float [[INDVAR_CONV]])
-; CHECK-NEXT:    [[CONV_I32:%.*]] = fptosi float [[INDVAR_CONV]] to i32
-; CHECK-NEXT:    call void @use.i32(i32 [[CONV_I32]])
+; CHECK-NEXT:    call void @use.i32(i32 [[FLOAT_IV_INT]])
 ; CHECK-NEXT:    [[CONV_I16:%.*]] = fptosi float [[INDVAR_CONV]] to i16
 ; CHECK-NEXT:    [[CONV_I64:%.*]] = fptosi float [[INDVAR_CONV]] to i64
 ; CHECK-NEXT:    call void @use.i16(i16 [[CONV_I16]])
Index: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
+++ llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
@@ -393,14 +393,26 @@
   // If the FP induction variable still has uses, this is because something else
   // in the loop uses its value.  In order to canonicalize the induction
   // variable, we chose to eliminate the IV and rewrite it in terms of an
-  // int->fp cast.
+  // int->fp cast. fp->int casts of the original induction can be directly
+  // simplified to use the integer induction .
   //
   // We give preference to sitofp over uitofp because it is faster on most
   // platforms.
   if (WeakPH) {
-    Value *Conv = new SIToFPInst(NewPHI, PN->getType(), "indvar.conv",
-                                 &*PN->getParent()->getFirstInsertionPt());
-    PN->replaceAllUsesWith(Conv);
+    Value *Conv = nullptr;
+    for (Use &U : make_early_inc_range(PN->uses())) {
+      auto *FPToSI = dyn_cast<FPToSIInst>(U.getUser());
+      if (FPToSI && FPToSI->getType() == NewPHI->getType()) {
+        FPToSI->replaceAllUsesWith(NewPHI);
+        DeadInsts.push_back(FPToSI);
+        continue;
+      }
+      if (!Conv)
+        Conv = new SIToFPInst(NewPHI, PN->getType(), "indvar.conv",
+                              &*PN->getParent()->getFirstInsertionPt());
+      U.set(Conv);
+    }
+
     RecursivelyDeleteTriviallyDeadInstructions(PN, TLI, MSSAU.get());
   }
   return true;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129140.442957.patch
Type: text/x-patch
Size: 2429 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220707/954d1c02/attachment.bin>