[instcombine] propagate single use freeze(gep inbounds X)

[reames]

Summary

This is a follow on for D111675 which implements the gep case. I'd originally left it out because I was hoping to actually implement the inrange todo, but after a bit of staring at the code, decided to leave it as is since it doesn't effect this use case (i.e. instcombine requires the op to freeze to be an instruction).

[reames]

llvm/lib/Analysis/ValueTracking.cpp

5059

For context here, inrange on a GEP operand is specified in a way which talks about UB triggered by uses. It doesn't explicitly state that poison is produced, but that appears to be the most consistent reading (we probably need to change LangRef).

inrange is only valid on constantexprs not instructions. However, this routine is written in terms of Operator, so in theory a constant expr could reach this point.

This issue exists in the previous code, I'm just adding the TODO since I reorganized it and was thinking about it.

[nikic]

LG

llvm/lib/Analysis/ValueTracking.cpp

5059

Yeah, this should be checking for inrange. Do you plan to follow up with that?

[reames]

llvm/lib/Analysis/ValueTracking.cpp

5059

If you're happy with a change without a test, sure. I haven't figured out how to exercise this yet.

I'm not signing up for the langref change though. :)

llvm/lib/Analysis/ValueTracking.cpp

if (ConsiderFlags) {
     // See whether I has flags that may create poison
     if (const auto *OvOp = dyn_cast<OverflowingBinaryOperator>(Op)) {
       if (OvOp->hasNoSignedWrap() || OvOp->hasNoUnsignedWrap())
         return true;
     }
     if (const auto *ExactOp = dyn_cast<PossiblyExactOperator>(Op))
       if (ExactOp->isExact())
         return true;
+    if (const auto *GEP = dyn_cast<GEPOperator>(Op))
+      if (GEP->isInBounds())
+        return true;
   }
 
   // TODO: this should really be under the ConsiderFlags block, but currently
   // these are not dropped by dropPoisonGeneratingFlags
   if (const auto *FP = dyn_cast<FPMathOperator>(Op)) {
     auto FMF = FP->getFastMathFlags();
     if (FMF.noNaNs() || FMF.noInfs())
       return true;
static bool canCreateUndefOrPoison(const Operator *Op, bool PoisonOnly,
   case Instruction::URem:
   case Instruction::SRem:
   case Instruction::ExtractValue:
   case Instruction::InsertValue:
   case Instruction::Freeze:
   case Instruction::ICmp:
   case Instruction::FCmp:
     return false;
-  case Instruction::GetElementPtr: {
-    const auto *GEP = cast<GEPOperator>(Op);
-    return GEP->isInBounds();
-  }
+  case Instruction::GetElementPtr:
+    // inbounds is handled above
+    // TODO: what about inrange on constexpr?
+    return false;
   default: {
     const auto *CE = dyn_cast<ConstantExpr>(Op);
     if (isa<CastInst>(Op) || (CE && CE->isCast()))
       return false;
     else if (Instruction::isBinaryOp(Opcode))
       return false;
     // Be conservative and return true.
     return true;

llvm/test/Transforms/InstCombine/freeze.ll

 ;
   %v1 = lshr exact i32 %arg, %unknown
   %v1.fr = freeze i32 %v1
   ret i32 %v1.fr
 }
 
 define i8* @propagate_drop_gep1(i8* %arg) {
 ; CHECK-LABEL: @propagate_drop_gep1(
-; CHECK-NEXT:    [[V1:%.*]] = getelementptr inbounds i8, i8* [[ARG:%.*]], i64 16
-; CHECK-NEXT:    [[V1_FR:%.*]] = freeze i8* [[V1]]
-; CHECK-NEXT:    ret i8* [[V1_FR]]
+; CHECK-NEXT:    [[ARG_FR:%.*]] = freeze i8* [[ARG:%.*]]
+; CHECK-NEXT:    [[V1:%.*]] = getelementptr i8, i8* [[ARG_FR]], i64 16
+; CHECK-NEXT:    ret i8* [[V1]]
 ;
   %v1 = getelementptr inbounds i8, i8* %arg, i64 16
   %v1.fr = freeze i8* %v1
   ret i8* %v1.fr
 }
 
 define float @propagate_drop_fneg(float %arg) {
 ; CHECK-LABEL: @propagate_drop_fneg(