Page MenuHomePhabricator

[IndVars] Smart hard uses detection
ClosedPublic

Authored by mkazantsev on Sep 3 2018, 2:42 AM.

Details

Summary

When rewriting loop exit values, IndVars considers this transform not profitable if
the loop instruction has a loop user which it believes cannot be optimized away.
In current implementation only calls that immediately use the instruction are considered
as such.

This patch extends the definition of "hard" users to any side-effecting instructions
(which usually cannot be optimized away from the loop) and also allows handling
of not just immediate users, but use chains.

Diff Detail

Event Timeline

etherzhhb added inline comments.Sep 18 2018, 9:13 AM
lib/Transforms/Scalar/IndVarSimplify.cpp
557–558

we may also stop and return true if Curr is a phi node, since phi nodes cannot be eliminated easily.

But we need to be careful that:

define void @test6(i32 %m, i32* %p) nounwind uwtable {
entry:
  br label %for.body

for.body:                                         ; preds = %for.body, %entry
  %i.06 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
  %a.05 = phi i32 [ 0, %entry ], [ %add, %for.body ]
  %inc = add nsw i32 %i.06, 1
  %exitcond = icmp eq i32 %inc, 186
  br i1 %exitcond, label %for.end, label %for.body

for.end:                                          ; preds = %for.body
  tail call void @func(i32 %inc)
  ret void
}

In this case we still what to rewrite %inc outside of the loop to a constant. (or it is handle by the other part of this pass?)

etherzhhb accepted this revision.Sep 18 2018, 9:20 AM

LGTM, but this patch may add a negligible increment on LLVM's compile time. Make sure you talk to some people that care about the LLVM compile time for their opinions

This revision is now accepted and ready to land.Sep 18 2018, 9:20 AM
mkazantsev added inline comments.Oct 31 2018, 3:34 AM
lib/Transforms/Scalar/IndVarSimplify.cpp
557–558

I guess constant values are always profitable to rewrite and don't need to pass through hard uses check. I'll take a look how exactly it's handled. It should not go on this check anyways.

cfang added a subscriber: cfang.Mar 25 2019, 2:11 PM

I am working on a regression caused by this patch. It is a memory access fault actually.
However, if we put back the original consition, my test would pass.

Could you explain why the original condition was removed in your patch?

+if (ExitValue->getSCEVType()>=scMulExpr) {

if (!isa<SCEVConstant>(ExitValue) && hasHardUserWithinLoop(L, Inst))
  continue;

+ }

I also found performance regression as a result of this commit:
http://lists.llvm.org/pipermail/llvm-dev/2019-March/131308.html