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

[fhahn]

Summary

When promoting a float induction to an integer one, we already proven
that all possible vales will be integers that can be converted back to
floats.

This makes fp -> int conversions of the float induction variable
redundant. Instead of unconditionally replacing all uses of the float
induction with a int -> fp conversion of the integer induction, simplify
fp -> int conversion to the integer induction directly.

Doing this in other passes would be a bit more tricky, as we would need
to prove the restricted range of the integer induction again separately.

Fixes #55505.

[nikic]

Related: https://reviews.llvm.org/D125774

[nikic]

llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

404

What guarantees that this is an fptosi to the correct type (i32 here)?

[fhahn]

In D129140#3630050, @nikic wrote:

Related: https://reviews.llvm.org/D125774

Argh, would have been good if this was mentioned in the issue...

Anyways I updated the code to restrict it to fptosi to types matching the integer phi

[fhahn]

llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

404

Ah right, fptosi can narrow or widen the result type. Should be fixed.

[reames]

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.

[Allen]

In 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.

[nikic]

llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

406

Add FPToSI to DeadInsts?

[fhahn]

Add fptosi to DeadInsts as suggested, thanks!

In D129140#3634418, @Allen wrote:

In 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!

[fhahn]

llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

406

Done, thanks!

[nikic]

As D129191 implements a more general solution, is it still worthwhile to also do this?

[nikic]

Don't think this is needed anymore.

[fhahn]

Yep, nothing left to do here.

llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

bool IndVarSimplify::handleFloatingPointIV(Loop *L, PHINode *PN) {
 
   // Delete the old floating point increment.
   Incr->replaceAllUsesWith(PoisonValue::get(Incr->getType()));
   RecursivelyDeleteTriviallyDeadInstructions(Incr, TLI, MSSAU.get());
 
   // 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",
+      U.set(Conv);
+    }
+
     RecursivelyDeleteTriviallyDeadInstructions(PN, TLI, MSSAU.get());
   }
   return true;
 }
 
 bool IndVarSimplify::rewriteNonIntegerIVs(Loop *L) {
   // First step.  Check to see if there are any floating-point recurrences.
   // If there are, change them into integer recurrences, permitting analysis by

llvm/test/Transforms/IndVarSimplify/floating-point-iv.ll

 define void @pr55505_remove_redundant_fptosi_for_float_iv(i32 %index, ptr %dst) {
 ; CHECK-LABEL: @pr55505_remove_redundant_fptosi_for_float_iv(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
 ; 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]])
 ; CHECK-NEXT:    call void @use.i64(i64 [[CONV_I64]])
 ; CHECK-NEXT:    [[UCONV_I16:%.*]] = fptoui float [[INDVAR_CONV]] to i16
 ; CHECK-NEXT:    [[UCONV_I64:%.*]] = fptoui float [[INDVAR_CONV]] to i64
 ; CHECK-NEXT:    call void @use.i16(i16 [[UCONV_I16]])
 ; CHECK-NEXT:    call void @use.i64(i64 [[UCONV_I64]])