[PATCH] D37121: [DivRemHoist] add a pass to move div/rem pairs into the same block (PR31028)

[Sanjay Patel via Phabricator via llvm-commits]

spatel updated this revision to Diff 113128.
spatel added a comment.
Herald added a subscriber: eraman.

Patch updated:

1. Use moveAfter() to simplify the hoisting code (convenience function added with https://reviews.llvm.org/rL312001).
2. Move the pass later in the pipeline. I noticed that the transform wasn't holding in loops (example below) because loop transforms will sink the sibling op to its use. That defeats the point of this pass, so this time I've made the transform really late, but still before the final -simplifycfg.
3. Add the pass to the new pass manager pipeline in the equivalent position. I failed to add the pass at all in the last rev (!).
4. Adding to the new pipeline means there are actually tests to check that the pass is running, so updated those.
5. Added preservation of GlobalsAA for both pipelines. The lack of this was causing a test failure in test/Transforms/PhaseOrdering/globalaa-retained.ll with the previous placement of the pass. I'm not sure if that would still happen now, but it's safer to make that explicit?

Here's an example of a loop with div/rem where we do not want another pass to re-sink the rem (because codegen can't fix that). Should I add a PhaseOrdering test like this?

  define void @rebase_mask(i32 %n, i32 %divisor, i32* %mask) {
  entry:
    %cmp = icmp sgt i32 %n, 0
    br i1 %cmp, label %preheader, label %exit
  
  preheader:
    br label %for.body
  
  exit:
    ret void
  
  for.body:
    %i = phi i32 [ %inc, %cleanup ], [ 0, %preheader ]
    %div = sdiv i32 %i, %divisor
    %idxprom = sext i32 %div to i64
    %arrayidx = getelementptr inbounds i32, i32* %mask, i64 %idxprom
    %ld = load i32, i32* %arrayidx, align 4
    %cmp1 = icmp slt i32 %ld, 0
    br i1 %cmp1, label %cleanup, label %if.end
  
  if.end:
    %rem = srem i32 %i, %divisor
    store i32 %rem, i32* %arrayidx, align 4
    br label %cleanup
  
  cleanup:
    %inc = add nuw nsw i32 %i, 1
    %exitcond = icmp eq i32 %inc, %n
    br i1 %exitcond, label %exit, label %for.body
  }


https://reviews.llvm.org/D37121

Files:
  include/llvm/InitializePasses.h
  include/llvm/Transforms/Scalar.h
  include/llvm/Transforms/Scalar/DivRemHoist.h
  lib/Passes/PassBuilder.cpp
  lib/Passes/PassRegistry.def
  lib/Transforms/IPO/PassManagerBuilder.cpp
  lib/Transforms/Scalar/CMakeLists.txt
  lib/Transforms/Scalar/DivRemHoist.cpp
  lib/Transforms/Scalar/Scalar.cpp
  test/Other/new-pm-defaults.ll
  test/Other/new-pm-thinlto-defaults.ll
  test/Transforms/DivRemHoist/div-rem-pairs.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D37121.113128.patch
Type: text/x-patch
Size: 20674 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170829/1417d625/attachment.bin>