[PATCH] D133198: [SCCP] convert signed div/rem to unsigned for non-negative operands

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D133198#3767857 <https://reviews.llvm.org/D133198#3767857>, @fhahn wrote:

>> I didn't find an explanation for skipping a constant operand in the existing zext code (ie, sext i8 42 -> zext i8 42 is valid, but we skip it). So that clause is not included for this transform, but there is a test diff for that pattern in case it should be bypassed.
>
> Do those operations have users left? If they can be simplified to a constant, I'd expect `tryToReplaceWithConstant` to take care of replacing all users of them.

Yes, the instruction still has a user. For example, when I step through this:

  define i16 @srem_cmp_constants() {
    %sel = select i1 false, i16 0, i16 12704
    %r = srem i16 %sel, 0
    ret i16 %r
  }

I see:

  % opt -ipsccp sdiv.ll -S -debug
  Args: opt -ipsccp sdiv.ll -S -debug 
  Marking Block Executable: 
  
  Popped off BBWL: 
    %sel = select i1 false, i16 0, i16 12704
    %r = srem i16 %sel, 0
    ret i16 %r
  
  Merged constantrange<12704, 12705> into   %sel = select i1 false, i16 0, i16 12704 : constantrange<12704, 12705>
  
  Popped off I-WL:   %sel = select i1 false, i16 0, i16 12704
  RESOLVING UNDEFS
  markOverdefined:   %r = srem i16 %sel, 0
  
  Popped off OI-WL:   %r = srem i16 %sel, 0
  Merged overdefined into define i16 @srem_cmp_constants() {
    %sel = select i1 false, i16 0, i16 12704
    %r = srem i16 %sel, 0
    ret i16 %r
  }
   : overdefined
  
  Popped off OI-WL: define i16 @srem_cmp_constants() {
    %sel = select i1 false, i16 0, i16 12704
    %r = srem i16 %sel, 0
    ret i16 %r
  }
  
  RESOLVING UNDEFS
    Constant: i16 12704 =   %sel = select i1 false, i16 0, i16 12704
  ; ModuleID = 'sdiv.ll'
  source_filename = "sdiv.ll"
  
  define i16 @srem_cmp_constants() {
    %r = srem i16 12704, 0
    ret i16 %r
  }

So we are doing the RAUW, but the solver is not re-run on those uses to get the subsequent constant-folding?


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

https://reviews.llvm.org/D133198