[PHITransAddr] Simplify casts in PHITransAddr

[kachkov98]

Summary

Try to simplify cast in similar way as for GEP and ADD with constant (e.g. sext/zext + trunc).

[mkazantsev]

llvm/lib/Analysis/PHITransAddr.cpp

198

I'm fine with isSafeToSpeculativelyExecute part, it seems reasonable. But this doesn't seem to be NFC while whole patch claims it's an NFC. Should it be 2 different patches?

[kachkov98]

llvm/lib/Analysis/PHITransAddr.cpp

198

I agree, after this change it can fold zext/sext + trunc for example (didn't see any changes on test-suite though).

[kachkov98]

Split patch

[mkazantsev]

llvm/lib/Analysis/PHITransAddr.cpp

198

Can you write a test yourself?

[mkazantsev]

Please add a test. If you can't figure it out yourself, you can insert an assert and run a big enough tests corps to see if something changed, and then use bugpoint to reduce it. Good luck!

[kachkov98]

Added synthetic test for simplifyCastInst

[kachkov98]

Separate patch for removing isSafeToSpeculativelyExecute() checks: https://reviews.llvm.org/D143255

[mkazantsev]

Looks reasonable. I'm not familiar with this code part in general, so please wait some time in case if someone has concerns, but looks pretty straightforward.

[kachkov98]

In D143167#4102182, @mkazantsev wrote:

Looks reasonable. I'm not familiar with this code part in general, so please wait some time in case if someone has concerns, but looks pretty straightforward.

Ok, thank you for the reviews!

llvm/lib/Analysis/PHITransAddr.cpp

if (CastInst *Cast = dyn_cast<CastInst>(Inst)) {
     if (!isSafeToSpeculativelyExecute(Cast)) return nullptr;
     Value *PHIIn = translateSubExpr(Cast->getOperand(0), CurBB, PredBB, DT);
     if (!PHIIn) return nullptr;
     if (PHIIn == Cast->getOperand(0))
       return Cast;
 
     // Find an available version of this cast.
 
-    // Constants are trivial to find.
-    if (Constant *C = dyn_cast<Constant>(PHIIn))
-      return addAsInput(
-          ConstantExpr::getCast(Cast->getOpcode(), C, Cast->getType()));
+    // Try to simplify cast first.
+    if (Value *V = simplifyCastInst(Cast->getOpcode(), PHIIn, Cast->getType(),
+                                    {DL, TLI, DT, AC})) {
+      RemoveInstInputs(PHIIn, InstInputs);
+      return addAsInput(V);
+    }
 
     // Otherwise we have to see if a casted version of the incoming pointer
     // is available.  If so, we can use it, otherwise we have to fail.
     for (User *U : PHIIn->users()) {
       if (CastInst *CastI = dyn_cast<CastInst>(U))
         if (CastI->getOpcode() == Cast->getOpcode() &&
             CastI->getType() == Cast->getType() &&
             (!DT || DT->dominates(CastI->getParent(), PredBB)))

llvm/test/Transforms/GVN/PRE/phi-translate-2.ll

while.end:
   ret void
 }
 
 ; Load from arrayidx2 is partially redundant, check that address translation can
 ; fold sext + trunc across phi node together.
 define i32 @test7(ptr noalias %ptr1, ptr noalias %ptr2, i32 %i, i1 %cond) {
 ; CHECK-LABEL: @test7(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br i1 [[COND:%.*]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+; CHECK-NEXT:    br i1 [[COND:%.*]], label [[IF_THEN:%.*]], label [[ENTRY_IF_END_CRIT_EDGE:%.*]]
+; CHECK:       entry.if.end_crit_edge:
+; CHECK-NEXT:    [[RES_PRE:%.*]] = load i32, ptr [[PTR1:%.*]], align 4
+; CHECK-NEXT:    br label [[IF_END:%.*]]
 ; CHECK:       if.then:
-; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[PTR1:%.*]], i32 [[I:%.*]]
+; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[PTR1]], i32 [[I:%.*]]
 ; CHECK-NEXT:    [[TMP:%.*]] = load i32, ptr [[ARRAYIDX]], align 4
 ; CHECK-NEXT:    store i32 [[TMP]], ptr [[PTR2:%.*]], align 4
 ; CHECK-NEXT:    [[IDX_EXT:%.*]] = sext i32 [[I]] to i64
 ; CHECK-NEXT:    br label [[IF_END]]
 ; CHECK:       if.end:
-; CHECK-NEXT:    [[IDX:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[IDX_EXT]], [[IF_THEN]] ]
+; CHECK-NEXT:    [[RES:%.*]] = phi i32 [ [[RES_PRE]], [[ENTRY_IF_END_CRIT_EDGE]] ], [ [[TMP]], [[IF_THEN]] ]
+; CHECK-NEXT:    [[IDX:%.*]] = phi i64 [ 0, [[ENTRY_IF_END_CRIT_EDGE]] ], [ [[IDX_EXT]], [[IF_THEN]] ]
 ; CHECK-NEXT:    [[IDX_TRUNC:%.*]] = trunc i64 [[IDX]] to i32
 ; CHECK-NEXT:    [[ARRAYIDX2:%.*]] = getelementptr inbounds i32, ptr [[PTR1]], i32 [[IDX_TRUNC]]
-; CHECK-NEXT:    [[RES:%.*]] = load i32, ptr [[ARRAYIDX2]], align 4
 ; CHECK-NEXT:    ret i32 [[RES]]
 ;
 entry:
   br i1 %cond, label %if.then, label %if.end
 
 if.then:
   %arrayidx = getelementptr inbounds i32, ptr %ptr1, i32 %i
   %tmp = load i32, ptr %arrayidx, align 4