[PATCH] D15559: [SCEVExpander] Make findExistingExpansion smarter

[Michael Zolotukhin via llvm-commits]

mzolotukhin requested changes to this revision.
mzolotukhin added a comment.
This revision now requires changes to proceed.

> Michael, could you make an example which is more complex that current clean up passes can't deal with? I checked your previous example's expensive expression is cleaned up.


Providing a concrete example here isn't even that important due to arguments I raised earlier, but here you go:

  @a = global i64 1
  
  declare void @foo()
  
  define i32 @test3(i64* %loc, i64 %conv7) {
  entry:
    %rem0 = load i64, i64* %loc, align 8
    %ExpensiveComputation = udiv i64 %rem0, 42
    br label %outer.header
  
  outer.header:
    %i = phi i64 [ 1, %entry ], [ %i.next, %outer.latch ]
    br label %bb1
  
  bb1:
    %addr = getelementptr i64, i64* @a, i64 %i
    %x = load volatile i64, i64* %addr
    %div11 = udiv i64 %x, %i
    %cmp.i38 = icmp ugt i64 %div11, 1
    %div12 = select i1 %cmp.i38, i64 %div11, i64 1
    br label %for.body
  
  for.body:
    %rem1 = phi i64 [ %rem0, %bb1 ], [ %rem2, %for.body ]
    %k1 = phi i64 [ %div12, %bb1 ], [ %dec, %for.body ]
    %mul1 = mul i64 %rem1, 48271
    %rem2 = urem i64 %mul1, 2147483647
    %dec = add i64 %k1, -1
    %cmp = icmp eq i64 %dec, 0
    br i1 %cmp, label %outer.latch, label %for.body
  
  outer.latch:
    %i.next = add i64 %i, 1
    %outer.cmp = icmp eq i64 %i.next, 1000
    br i1 %outer.cmp, label %exit, label %outer.header
  
  exit:
    %rem3 = phi i64 [ %rem2, %outer.latch ]
    store i64 %rem3, i64* %loc, align 8
    ret i32 0
  }

I ran it with

  opt < test.ll -S -unroll-runtime -loop-unroll -early-cse -instcombine -simplifycfg -licm

and always saw a code like this in the output:

  %x = load volatile i64, i64* %addr, align 8
  %div11 = udiv i64 %x, %i
  %cmp.i38 = icmp ugt i64 %div11, 1
  %div12 = select i1 %cmp.i38, i64 %div11, i64 1
  %1 = udiv i64 %x, %0                           ; <<< Redundant expensive computation
  %2 = icmp ugt i64 %1, 1
  %umax = select i1 %2, i64 %1, i64 1

Probably, you can come up with another set of passes that will be able to clean this up, but then the example might be made even more complicated, so the new set will fail. The problem here is that with this patch compiler thinks that it generates no redundant code to clean-up, while in fact it's not even limited in what it can generate - we expand *any* existing SCEV expression.

So, I'd suggest you to address the issue with `expand` before going further with this one.

Michael


http://reviews.llvm.org/D15559