[ScalarEvolution] Fix pointer/int type handling converting select/phi to min/max.

[efriedma]

Summary

The old version of this code would blindly perform arithmetic without paying attention to whether the types involved were pointers or integers. This could lead to weird expressions like negating a pointer.

Explicitly handle simple cases involving pointers, like "x < y ? x : y". In all other cases, coerce the operands of the comparison to integer types. This avoids the weird cases, while handling most of the interesting cases.

[lebedev.ri]

I like it!

[xbolva00]

llvm/test/Transforms/IndVarSimplify/pr45835.ll

29

remove then?

[reames]

Can you take a step back and describe the problem you're trying to solve? I've read through the bug, but nothing there immediately makes me think "bug in core part of SCEV" as opposed to "bug in user of SCEV". How'd you get from there to here?

This patch as implemented seems to be a strict regression. Without understanding what symptom you're worried about, it's hard to tell if this is a reasonable solution.

Given this seems to be related to a release regression and there may be a tendency to rush this, let me be really explicit. This, and the change it's based on, are not okay to land without either my sign off or from someone else who regularly works on SCEV (e.g. @mkazantsev, @nikic). This is a deep change which has non-obvious implications, with unclear motivation. Currently, I don't think either of these changes should land at all, much less in a hurry.

JFYI, I'm going to be offline for the next two days, I'll return to this Monday.

[efriedma]

I'm happy to take some time to discuss this; if we urgently some other solution for the branch, we can take a more targeted approach.

The ultimate goal here is to make SCEV consistent about the "pointer-ness" of a SCEV value. If you call getSCEV() on a value, or evaluate a SCEV value at a particular iteration, or something along those lines, the output should be a pointer if and only if the input is a pointer. And the input and output should have the same pointer base. Enforcing these restrictions will make it easier to preserve correctness, and to reason about values like non-integral pointers.

As far as I can tell, we're pretty close. This patch plus D103656 handle almost all the interesting cases. The one remaining issue after these two patches is code outside SCEV that explicitly constructs min/max nodes using pointer values. I have a WIP patch for that which seems to pass tests; I'll try to post some time next week.

[reames]

Eli,

The goal stated of having getSCEV(V)->getType()->isPointerTy() == V->getType()->isPointerTy() seems reasonable to me. I'm not as sure about the baseof(V) == baseof(S) bit, but I tentatively accept that as it's not my main confusion.

My main confusion is why tackle the problem this way? If we're constructing a SCEV node for an existing IR instruction which has two base pointers involved, then a) we've got a question about what the semantics of that instruction are at all (e.g. it's probably poison), and b) the SCEV result seems like it should have the same set of base pointers as the instruction.

Or, maybe said another way, what makes selects special here?

Can I maybe suggest you split this into a patch which enforces the pointerness property, and then a patch which imposes the baseof property? The former seems easy-ish to assert in getSCEV(V) and enumerate the cases which violate.

[efriedma]

Here's a possible algorithm for determining pointer-ness of a SCEV expression:

1. SCEVUnknown is a pointer if and only if the LLVM IR value is a pointer.
2. SCEVPtrToInt is never a pointer.
3. If any other SCEV expression has no pointer operands, the result is an integer.
4. If a SCEVAddExpr has exactly one pointer operand, the result is a pointer.
5. If a SCEVAddRecExpr's first operand is a pointer, and it has no other pointer operands, the result is a pointer.
6. Otherwise, the SCEV expression is invalid.

Most of the results that come out of this algorithm aren't really controversial. It doesn't make sense to multiply by a pointer, or divide by a pointer, or add two pointers to each other.

We could possibly add a rule like "If a SCEVMinMaxExpr has all pointer operands, the result is a pointer". When I was looking at this originally, I was under the impression this would be problematic due to existing SCEV transforms, but looking again, maybe it's okay; I somehow thought the SCEV getter operations were more aggressive than they actually are. If it does work out, that would allow me to narrow the scope of this patch to some extent.

In any case, the issue with createNodeForSelectOrPHI is that it likes to create expressions like, for example, ((-1 * %p) + ((1 + %p) umax (2 + %p))), where %p is a pointer. (https://bugs.llvm.org/show_bug.cgi?id=46786#c17) This breaks any reasonable version of the above rules.

I'll mess with this a bit more.

[efriedma]

Switching patch to a version that preserves most of the analysis power, while still avoiding the weird cases.

[reames]

The revised change looks a lot more reasonable. With some cleanup, I'd be willing to LGTM this. I'm much happier with the explicit focus on avoiding the construction of a subtract of pointer type.

You combined the logic for the signed and unsigned case. Can I ask that you commit an NFC change which does that, and then rebase this? The overall structure is fine for the NFC version, I just want the diffs to stand out in this change.

Your suggested rules in the previous comment also seem to be a reasonable direction. My concern is around the subtract case. As you've noted, there's a bunch of cases where we subtract pointers today, and finding variants for those is going to taken some work. I'd also *strongly* encourage you to encode your rules as assertions. :)

Hm, have you considered doing the coercion check inside getMinusSCEV? If the construct we're trying to outlaw is a subtract of pointers, maybe we should just explicitly do that? (I'm fine with a cleaned up version of this landing, then exploring that if desired.)

llvm/lib/Analysis/ScalarEvolution.cpp

5554

Repeated code, masking a variable.

llvm/test/Transforms/IndVarSimplify/pr45835.ll

13

Can you explain this test change?

[efriedma]

Hm, have you considered doing the coercion check inside getMinusSCEV? If the construct we're trying to outlaw is a subtract of pointers, maybe we should just explicitly do that? (I'm fine with a cleaned up version of this landing, then exploring that if desired.)

I'll thin about it.

llvm/test/Transforms/IndVarSimplify/pr45835.ll

13

The select was getting matched as a umax; the current version of the logic can't handle that.

[reames]

llvm/test/Transforms/IndVarSimplify/pr45835.ll

13

Ok.

[efriedma]

Address review comments.

[reames]

LGTM

llvm/lib/Analysis/ScalarEvolution.cpp

if (!ICI)
     return getUnknown(I);
 
   Value *LHS = ICI->getOperand(0);
   Value *RHS = ICI->getOperand(1);
 
   switch (ICI->getPredicate()) {
   case ICmpInst::ICMP_SLT:
   case ICmpInst::ICMP_SLE:
-    std::swap(LHS, RHS);
-    LLVM_FALLTHROUGH;
-  case ICmpInst::ICMP_SGT:
-  case ICmpInst::ICMP_SGE:
-    // a >s b ? a+x : b+x  ->  smax(a, b)+x
-    // a >s b ? b+x : a+x  ->  smin(a, b)+x
-    if (getTypeSizeInBits(LHS->getType()) <= getTypeSizeInBits(I->getType())) {
-      const SCEV *LS = getNoopOrSignExtend(getSCEV(LHS), I->getType());
-      const SCEV *RS = getNoopOrSignExtend(getSCEV(RHS), I->getType());
-      const SCEV *LA = getSCEV(TrueVal);
-      const SCEV *RA = getSCEV(FalseVal);
-      const SCEV *LDiff = getMinusSCEV(LA, LS);
-      const SCEV *RDiff = getMinusSCEV(RA, RS);
-      if (LDiff == RDiff)
-        return getAddExpr(getSMaxExpr(LS, RS), LDiff);
-      LDiff = getMinusSCEV(LA, RS);
-      RDiff = getMinusSCEV(RA, LS);
-      if (LDiff == RDiff)
-        return getAddExpr(getSMinExpr(LS, RS), LDiff);
-    }
-    break;
   case ICmpInst::ICMP_ULT:
   case ICmpInst::ICMP_ULE:
     std::swap(LHS, RHS);
     LLVM_FALLTHROUGH;
+  case ICmpInst::ICMP_SGT:
+  case ICmpInst::ICMP_SGE:
   case ICmpInst::ICMP_UGT:
   case ICmpInst::ICMP_UGE:
-    // a >u b ? a+x : b+x  ->  umax(a, b)+x
-    // a >u b ? b+x : a+x  ->  umin(a, b)+x
+    // a > b ? a+x : b+x  ->  max(a, b)+x
+    // a > b ? b+x : a+x  ->  min(a, b)+x
     if (getTypeSizeInBits(LHS->getType()) <= getTypeSizeInBits(I->getType())) {
-      const SCEV *LS = getNoopOrZeroExtend(getSCEV(LHS), I->getType());
-      const SCEV *RS = getNoopOrZeroExtend(getSCEV(RHS), I->getType());
       const SCEV *LA = getSCEV(TrueVal);
       const SCEV *RA = getSCEV(FalseVal);
+      const SCEV *LS = getSCEV(LHS);
+      const SCEV *RS = getSCEV(RHS);
+      if (LA->getType()->isPointerTy()) {
+        // FIXME: Handle cases where LS/RS are pointers not equal to LA/RA.
+        // Need to make sure we can't produce weird expressions involving
+        // negated pointers.
+        const SCEV *LS = getSCEV(LHS);
+        const SCEV *RS = getSCEV(RHS);
+        if (LA == LS && RA == RS)
+          return ICI->isSigned() ? getSMaxExpr(LS, RS) : getUMaxExpr(LS, RS);
+        if (LA == RS && RA == LS)
+          return ICI->isSigned() ? getSMinExpr(LS, RS) : getUMinExpr(LS, RS);
+      }
+      auto CoerceOperand = [&](const SCEV *Op) -> const SCEV * {
+        if (Op->getType()->isPointerTy()) {
+          Op = getLosslessPtrToIntExpr(Op);
+          if (isa<SCEVCouldNotCompute>(Op))
+            return Op;
+        }
+        if (ICI->isSigned())
+          Op = getNoopOrSignExtend(Op, I->getType());
+        else
+          Op = getNoopOrZeroExtend(Op, I->getType());
+        return Op;
+      };
+      LS = CoerceOperand(LS);
+      RS = CoerceOperand(RS);
+      if (isa<SCEVCouldNotCompute>(LS) || isa<SCEVCouldNotCompute>(RS))
+        break;
       const SCEV *LDiff = getMinusSCEV(LA, LS);
       const SCEV *RDiff = getMinusSCEV(RA, RS);
       if (LDiff == RDiff)
-        return getAddExpr(getUMaxExpr(LS, RS), LDiff);
+        return getAddExpr(
+            ICI->isSigned() ? getSMaxExpr(LS, RS) : getUMaxExpr(LS, RS), LDiff);
       LDiff = getMinusSCEV(LA, RS);
       RDiff = getMinusSCEV(RA, LS);
       if (LDiff == RDiff)
-        return getAddExpr(getUMinExpr(LS, RS), LDiff);
+        return getAddExpr(
+            ICI->isSigned() ? getSMinExpr(LS, RS) : getUMinExpr(LS, RS), LDiff);
     }
     break;
   case ICmpInst::ICMP_NE:
     // n != 0 ? n+x : 1+x  ->  umax(n, 1)+x
     if (getTypeSizeInBits(LHS->getType()) <= getTypeSizeInBits(I->getType()) &&
         isa<ConstantInt>(RHS) && cast<ConstantInt>(RHS)->isZero()) {
       const SCEV *One = getOne(I->getType());
       const SCEV *LS = getNoopOrZeroExtend(getSCEV(LHS), I->getType());

llvm/test/Analysis/ScalarEvolution/pr46786.ll

 ; CHECK-NEXT:  Classifying expressions for: @FSE_decompress_usingDTable
 ; CHECK-NEXT:    %i = getelementptr inbounds i8, i8* %arg, i32 %arg2
 ; CHECK-NEXT:    --> (%arg2 + %arg) U: full-set S: full-set
 ; CHECK-NEXT:    %i4 = sub nsw i32 0, %arg1
 ; CHECK-NEXT:    --> (-1 * %arg1) U: full-set S: full-set
 ; CHECK-NEXT:    %i5 = getelementptr inbounds i8, i8* %i, i32 %i4
 ; CHECK-NEXT:    --> ((-1 * %arg1) + %arg2 + %arg) U: full-set S: full-set
 ; CHECK-NEXT:    %i7 = select i1 %i6, i32 %arg2, i32 %arg1
-; CHECK-NEXT:    --> ((-1 * %arg) + (((-1 * %arg1) + %arg2 + %arg) umin %arg) + %arg1) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * (ptrtoint i8* %arg to i32)) + (((-1 * %arg1) + (ptrtoint i8* %arg to i32) + %arg2) umin (ptrtoint i8* %arg to i32)) + %arg1) U: full-set S: full-set
 ; CHECK-NEXT:    %i8 = sub i32 %arg3, %i7
-; CHECK-NEXT:    --> ((-1 * (((-1 * %arg1) + %arg2 + %arg) umin %arg)) + (-1 * %arg1) + %arg3 + %arg) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * (((-1 * %arg1) + (ptrtoint i8* %arg to i32) + %arg2) umin (ptrtoint i8* %arg to i32))) + (-1 * %arg1) + (ptrtoint i8* %arg to i32) + %arg3) U: full-set S: full-set
 ; CHECK-NEXT:    %i9 = getelementptr inbounds i8, i8* %arg, i32 %i8
-; CHECK-NEXT:    --> ((2 * %arg) + (-1 * (((-1 * %arg1) + %arg2 + %arg) umin %arg)) + (-1 * %arg1) + %arg3) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * (((-1 * %arg1) + (ptrtoint i8* %arg to i32) + %arg2) umin (ptrtoint i8* %arg to i32))) + (-1 * %arg1) + (ptrtoint i8* %arg to i32) + %arg3 + %arg) U: full-set S: full-set
 ; CHECK-NEXT:  Determining loop execution counts for: @FSE_decompress_usingDTable
 ;
 bb:
   %i = getelementptr inbounds i8, i8* %arg, i32 %arg2
   %i4 = sub nsw i32 0, %arg1
   %i5 = getelementptr inbounds i8, i8* %i, i32 %i4
   %i6 = icmp ult i8* %i5, %arg
   %i7 = select i1 %i6, i32 %arg2, i32 %arg1
   %i8 = sub i32 %arg3, %i7
   %i9 = getelementptr inbounds i8, i8* %arg, i32 %i8
   ret i8* %i9
 }
 
 define i8* @test_01(i8* %p) {
 ; CHECK-LABEL: 'test_01'
 ; CHECK-NEXT:  Classifying expressions for: @test_01
 ; CHECK-NEXT:    %p1 = getelementptr i8, i8* %p, i32 2
 ; CHECK-NEXT:    --> (2 + %p) U: full-set S: full-set
 ; CHECK-NEXT:    %p2 = getelementptr i8, i8* %p, i32 1
 ; CHECK-NEXT:    --> (1 + %p) U: full-set S: full-set
 ; CHECK-NEXT:    %index = select i1 %cmp, i32 2, i32 1
-; CHECK-NEXT:    --> ((-1 * %p) + ((1 + %p) umax (2 + %p))) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * (ptrtoint i8* %p to i32)) + ((1 + (ptrtoint i8* %p to i32)) umax (2 + (ptrtoint i8* %p to i32)))) U: full-set S: full-set
 ; CHECK-NEXT:    %neg_index = sub i32 0, %index
-; CHECK-NEXT:    --> ((-1 * ((1 + %p) umax (2 + %p))) + %p) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * ((1 + (ptrtoint i8* %p to i32)) umax (2 + (ptrtoint i8* %p to i32)))) + (ptrtoint i8* %p to i32)) U: full-set S: full-set
 ; CHECK-NEXT:    %gep = getelementptr i8, i8* %p, i32 %neg_index
-; CHECK-NEXT:    --> ((2 * %p) + (-1 * ((1 + %p) umax (2 + %p)))) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * ((1 + (ptrtoint i8* %p to i32)) umax (2 + (ptrtoint i8* %p to i32)))) + (ptrtoint i8* %p to i32) + %p) U: full-set S: full-set
 ; CHECK-NEXT:  Determining loop execution counts for: @test_01
 ;
   %p1 = getelementptr i8, i8* %p, i32 2
   %p2 = getelementptr i8, i8* %p, i32 1
   %cmp = icmp ugt i8* %p1, %p2
   %index = select i1 %cmp, i32 2, i32 1
   %neg_index = sub i32 0, %index
   %gep = getelementptr i8, i8* %p, i32 %neg_index
   ret i8* %gep
 }
 
 define i8* @test_02(i8* %p) {
 ; CHECK-LABEL: 'test_02'
 ; CHECK-NEXT:  Classifying expressions for: @test_02
 ; CHECK-NEXT:    %p1 = getelementptr i8, i8* %p, i32 2
 ; CHECK-NEXT:    --> (2 + %p) U: full-set S: full-set
 ; CHECK-NEXT:    %p2 = getelementptr i8, i8* %p, i32 1
 ; CHECK-NEXT:    --> (1 + %p) U: full-set S: full-set
 ; CHECK-NEXT:    %index = select i1 %cmp, i32 2, i32 1
-; CHECK-NEXT:    --> ((-1 * %p) + ((1 + %p) smax (2 + %p))) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * (ptrtoint i8* %p to i32)) + ((1 + (ptrtoint i8* %p to i32)) smax (2 + (ptrtoint i8* %p to i32)))) U: full-set S: full-set
 ; CHECK-NEXT:    %neg_index = sub i32 0, %index
-; CHECK-NEXT:    --> ((-1 * ((1 + %p) smax (2 + %p))) + %p) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * ((1 + (ptrtoint i8* %p to i32)) smax (2 + (ptrtoint i8* %p to i32)))) + (ptrtoint i8* %p to i32)) U: full-set S: full-set
 ; CHECK-NEXT:    %gep = getelementptr i8, i8* %p, i32 %neg_index
-; CHECK-NEXT:    --> ((2 * %p) + (-1 * ((1 + %p) smax (2 + %p)))) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * ((1 + (ptrtoint i8* %p to i32)) smax (2 + (ptrtoint i8* %p to i32)))) + (ptrtoint i8* %p to i32) + %p) U: full-set S: full-set
 ; CHECK-NEXT:  Determining loop execution counts for: @test_02
 ;
   %p1 = getelementptr i8, i8* %p, i32 2
   %p2 = getelementptr i8, i8* %p, i32 1
   %cmp = icmp sgt i8* %p1, %p2
   %index = select i1 %cmp, i32 2, i32 1
   %neg_index = sub i32 0, %index
   %gep = getelementptr i8, i8* %p, i32 %neg_index
   ret i8* %gep
 }
 
 define i8* @test_03(i8* %p) {
 ; CHECK-LABEL: 'test_03'
 ; CHECK-NEXT:  Classifying expressions for: @test_03
 ; CHECK-NEXT:    %p1 = getelementptr i8, i8* %p, i32 2
 ; CHECK-NEXT:    --> (2 + %p) U: full-set S: full-set
 ; CHECK-NEXT:    %p2 = getelementptr i8, i8* %p, i32 1
 ; CHECK-NEXT:    --> (1 + %p) U: full-set S: full-set
 ; CHECK-NEXT:    %index = select i1 %cmp, i32 2, i32 1
-; CHECK-NEXT:    --> ((-1 * %p) + ((1 + %p) umin (2 + %p))) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * (ptrtoint i8* %p to i32)) + ((1 + (ptrtoint i8* %p to i32)) umin (2 + (ptrtoint i8* %p to i32)))) U: full-set S: full-set
 ; CHECK-NEXT:    %neg_index = sub i32 0, %index
-; CHECK-NEXT:    --> ((-1 * ((1 + %p) umin (2 + %p))) + %p) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * ((1 + (ptrtoint i8* %p to i32)) umin (2 + (ptrtoint i8* %p to i32)))) + (ptrtoint i8* %p to i32)) U: full-set S: full-set
 ; CHECK-NEXT:    %gep = getelementptr i8, i8* %p, i32 %neg_index
-; CHECK-NEXT:    --> ((2 * %p) + (-1 * ((1 + %p) umin (2 + %p)))) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * ((1 + (ptrtoint i8* %p to i32)) umin (2 + (ptrtoint i8* %p to i32)))) + (ptrtoint i8* %p to i32) + %p) U: full-set S: full-set
 ; CHECK-NEXT:  Determining loop execution counts for: @test_03
 ;
   %p1 = getelementptr i8, i8* %p, i32 2
   %p2 = getelementptr i8, i8* %p, i32 1
   %cmp = icmp ult i8* %p1, %p2
   %index = select i1 %cmp, i32 2, i32 1
   %neg_index = sub i32 0, %index
   %gep = getelementptr i8, i8* %p, i32 %neg_index
   ret i8* %gep
 }
 
 define i8* @test_04(i8* %p) {
 ; CHECK-LABEL: 'test_04'
 ; CHECK-NEXT:  Classifying expressions for: @test_04
 ; CHECK-NEXT:    %p1 = getelementptr i8, i8* %p, i32 2
 ; CHECK-NEXT:    --> (2 + %p) U: full-set S: full-set
 ; CHECK-NEXT:    %p2 = getelementptr i8, i8* %p, i32 1
 ; CHECK-NEXT:    --> (1 + %p) U: full-set S: full-set
 ; CHECK-NEXT:    %index = select i1 %cmp, i32 2, i32 1
-; CHECK-NEXT:    --> ((-1 * %p) + ((1 + %p) smin (2 + %p))) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * (ptrtoint i8* %p to i32)) + ((1 + (ptrtoint i8* %p to i32)) smin (2 + (ptrtoint i8* %p to i32)))) U: full-set S: full-set
 ; CHECK-NEXT:    %neg_index = sub i32 0, %index
-; CHECK-NEXT:    --> ((-1 * ((1 + %p) smin (2 + %p))) + %p) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * ((1 + (ptrtoint i8* %p to i32)) smin (2 + (ptrtoint i8* %p to i32)))) + (ptrtoint i8* %p to i32)) U: full-set S: full-set
 ; CHECK-NEXT:    %gep = getelementptr i8, i8* %p, i32 %neg_index
-; CHECK-NEXT:    --> ((2 * %p) + (-1 * ((1 + %p) smin (2 + %p)))) U: full-set S: full-set
+; CHECK-NEXT:    --> ((-1 * ((1 + (ptrtoint i8* %p to i32)) smin (2 + (ptrtoint i8* %p to i32)))) + (ptrtoint i8* %p to i32) + %p) U: full-set S: full-set
 ; CHECK-NEXT:  Determining loop execution counts for: @test_04
 ;
   %p1 = getelementptr i8, i8* %p, i32 2
   %p2 = getelementptr i8, i8* %p, i32 1
   %cmp = icmp slt i8* %p1, %p2
   %index = select i1 %cmp, i32 2, i32 1
   %neg_index = sub i32 0, %index
   %gep = getelementptr i8, i8* %p, i32 %neg_index
   ret i8* %gep
 }
 
 attributes #0 = { nofree }

llvm/test/Transforms/IndVarSimplify/pr45835.ll

 ; RUN: opt < %s -indvars -replexitval=always -S | FileCheck %s --check-prefix=ALWAYS
 ; RUN: opt < %s -indvars -replexitval=never -S | FileCheck %s --check-prefix=NEVER
 ; RUN: opt < %s -indvars -replexitval=cheap -scev-cheap-expansion-budget=1 -S | FileCheck %s --check-prefix=CHEAP
 
 ; rewriteLoopExitValues() must rewrite all or none of a PHI's values from a given block.
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
 @a = common global i8 0, align 1
 
 define internal fastcc void @d(i8* %c) unnamed_addr #0 {
 entry:
-  %cmp = icmp ule i8* %c, getelementptr inbounds (i8, i8* @a, i64 65535)
+  %cmp = icmp ule i8* %c, @a
   %add.ptr = getelementptr inbounds i8, i8* %c, i64 -65535
   br label %while.cond
 
 while.cond:
   br i1 icmp ne (i8 0, i8 0), label %cont, label %while.end
 
 cont:
-  %a.mux = select i1 %cmp, i8* @a, i8* %add.ptr
+  %a.mux = select i1 %cmp, i8* @a, i8* %c
   switch i64 0, label %while.cond [
     i64 -1, label %handler.pointer_overflow.i
     i64 0, label %handler.pointer_overflow.i
   ]
 
 handler.pointer_overflow.i:
   %a.mux.lcssa4 = phi i8* [ %a.mux, %cont ], [ %a.mux, %cont ]
-; ALWAYS: [ %scevgep, %cont ], [ %scevgep, %cont ]
+; ALWAYS: [ %umax, %cont ], [ %umax, %cont ]
 ; NEVER: [ %a.mux, %cont ], [ %a.mux, %cont ]
 ; In cheap mode, use either one as long as it's consistent.
 ; CHEAP: [ %[[VAL:.*]], %cont ], [ %[[VAL]], %cont ]
   %x5 = ptrtoint i8* %a.mux.lcssa4 to i64
   br label %while.end
 
 while.end:
   ret void
 }

llvm/unittests/Transforms/Utils/ScalarEvolutionExpanderTest.cpp

TEST_F(ScalarEvolutionExpanderTest, ExpandPtrTypeSCEV) {
   CmpInst *Cmp = CmpInst::Create(Instruction::ICmp, CmpInst::ICMP_ULT,
                                  UndefValue::get(I8PtrTy), CastA, "cmp", Br);
   SelectInst *Sel = SelectInst::Create(Cmp, Gep1, Gep2, "select", Br);
   CastInst *CastB =
       CastInst::CreateBitOrPointerCast(Sel, I32PtrTy, "bitcast2", Br);
 
   ScalarEvolution SE = buildSE(*F);
   auto *S = SE.getSCEV(CastB);
-  SCEVExpander Exp(SE, M.getDataLayout(), "expander");
-  Value *V =
-      Exp.expandCodeFor(cast<SCEVAddExpr>(S)->getOperand(1), nullptr, Br);
-
-  // Expect the expansion code contains:
-  //   %0 = bitcast i32* %bitcast2 to i8*
-  //   %uglygep = getelementptr i8, i8* %0, i64 -1
-  //   %1 = bitcast i8* %uglygep to i32*
-  EXPECT_TRUE(isa<BitCastInst>(V));
-  Instruction *Gep = cast<Instruction>(V)->getPrevNode();
-  EXPECT_TRUE(isa<GetElementPtrInst>(Gep));
-  EXPECT_TRUE(isa<ConstantInt>(Gep->getOperand(1)));
-  EXPECT_EQ(cast<ConstantInt>(Gep->getOperand(1))->getSExtValue(), -1);
-  EXPECT_TRUE(isa<BitCastInst>(Gep->getPrevNode()));
+  EXPECT_TRUE(isa<SCEVUnknown>(S));
 }
 
 // Make sure that SCEV doesn't introduce illegal ptrtoint/inttoptr instructions
 TEST_F(ScalarEvolutionExpanderTest, SCEVZeroExtendExprNonIntegral) {
   /*
    * Create the following code:
    * func(i64 addrspace(10)* %arg)
    * top: