[llvm] r345814 - [IndVars] Smart hard uses detection
Mikael Holmén via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 2 05:19:44 PDT 2018
Hi Max,
I'm seeing something that I think is an unfortunate result of this change.
If I run indvars on foo.ll with this change:
opt -S -indvars -o - foo.ll
I get:
define i16 @foo() {
entry:
br label %for.body
for.body: ; preds = %for.body,
%entry
%i = phi i16 [ 0, %entry ], [ %inc, %for.body ]
%arrayidx = getelementptr inbounds [400 x i16], [400 x i16]* @Y, i16
0, i16 %i
store i16 0, i16* %arrayidx, align 1
%inc = add nuw nsw i16 %i, 1
%cmp = icmp ult i16 %inc, 400
br i1 %cmp, label %for.body, label %for.end
for.end: ; preds = %for.body
%inc.lcssa = phi i16 [ %inc, %for.body ]
ret i16 %inc.lcssa
}
but without the change I get:
define i16 @foo() {
entry:
br label %for.body
for.body: ; preds = %for.body,
%entry
%i = phi i16 [ 0, %entry ], [ %inc, %for.body ]
%arrayidx = getelementptr inbounds [400 x i16], [400 x i16]* @Y, i16
0, i16 %i
store i16 0, i16* %arrayidx, align 1
%inc = add nuw nsw i16 %i, 1
%cmp = icmp ult i16 %inc, 400
br i1 %cmp, label %for.body, label %for.end
for.end: ; preds = %for.body
ret i16 400
}
So before we concluded that the return value was always 400, but now we
don't? Is this really what we want with this change?
Regards,
Mikael
On 11/1/18 7:47 AM, Max Kazantsev via llvm-commits wrote:
> Author: mkazantsev
> Date: Wed Oct 31 23:47:01 2018
> New Revision: 345814
>
> URL: http://llvm.org/viewvc/llvm-project?rev=345814&view=rev
> Log:
> [IndVars] Smart hard uses detection
>
> 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.
>
> Differentlai Revision: https://reviews.llvm.org/D51584
> Reviewed By: etherzhhb
>
> Modified:
> llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
> llvm/trunk/test/Analysis/ScalarEvolution/pr28705.ll
> llvm/trunk/test/Transforms/IndVarSimplify/dont-recompute.ll
> llvm/trunk/test/Transforms/IndVarSimplify/lrev-existing-umin.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp?rev=345814&r1=345813&r2=345814&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp Wed Oct 31 23:47:01 2018
> @@ -145,6 +145,7 @@ class IndVarSimplify {
> bool canLoopBeDeleted(Loop *L, SmallVector<RewritePhi, 8> &RewritePhiSet);
> bool rewriteLoopExitValues(Loop *L, SCEVExpander &Rewriter);
> bool rewriteFirstIterationLoopExitValues(Loop *L);
> + bool hasHardUserWithinLoop(const Loop *L, const Instruction *I) const;
>
> bool linearFunctionTestReplace(Loop *L, const SCEV *BackedgeTakenCount,
> PHINode *IndVar, SCEVExpander &Rewriter);
> @@ -524,6 +525,29 @@ struct RewritePhi {
> // As a side effect, reduces the amount of IV processing within the loop.
> //===----------------------------------------------------------------------===//
>
> +bool IndVarSimplify::hasHardUserWithinLoop(const Loop *L, const Instruction *I) const {
> + SmallPtrSet<const Instruction *, 8> Visited;
> + SmallVector<const Instruction *, 8> WorkList;
> + Visited.insert(I);
> + WorkList.push_back(I);
> + while (!WorkList.empty()) {
> + const Instruction *Curr = WorkList.pop_back_val();
> + // This use is outside the loop, nothing to do.
> + if (!L->contains(Curr))
> + continue;
> + // Do we assume it is a "hard" use which will not be eliminated easily?
> + if (Curr->mayHaveSideEffects())
> + return true;
> + // Otherwise, add all its users to worklist.
> + for (auto U : Curr->users()) {
> + auto *UI = cast<Instruction>(U);
> + if (Visited.insert(UI).second)
> + WorkList.push_back(UI);
> + }
> + }
> + return false;
> +}
> +
> /// Check to see if this loop has a computable loop-invariant execution count.
> /// If so, this means that we can compute the final value of any expressions
> /// that are recurrent in the loop, and substitute the exit values from the loop
> @@ -598,19 +622,8 @@ bool IndVarSimplify::rewriteLoopExitValu
> // Computing the value outside of the loop brings no benefit if it is
> // definitely used inside the loop in a way which can not be optimized
> // away.
> - if (ExitValue->getSCEVType()>=scMulExpr) {
> - bool HasHardInternalUses = false;
> - for (auto *IB : Inst->users()) {
> - Instruction *UseInstr = cast<Instruction>(IB);
> - unsigned Opc = UseInstr->getOpcode();
> - if (L->contains(UseInstr) && Opc == Instruction::Call) {
> - HasHardInternalUses = true;
> - break;
> - }
> - }
> - if (HasHardInternalUses)
> - continue;
> - }
> + if (hasHardUserWithinLoop(L, Inst))
> + continue;
>
> bool HighCost = Rewriter.isHighCostExpansion(ExitValue, L, Inst);
> Value *ExitVal = Rewriter.expandCodeFor(ExitValue, PN->getType(), Inst);
>
> Modified: llvm/trunk/test/Analysis/ScalarEvolution/pr28705.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/pr28705.ll?rev=345814&r1=345813&r2=345814&view=diff
> ==============================================================================
> --- llvm/trunk/test/Analysis/ScalarEvolution/pr28705.ll (original)
> +++ llvm/trunk/test/Analysis/ScalarEvolution/pr28705.ll Wed Oct 31 23:47:01 2018
> @@ -1,11 +1,11 @@
> ; PR28705
> ; RUN: opt < %s -indvars -S | FileCheck %s
>
> -; Check IndVarSimplify replaces the exitval use of the induction var "%inc.i.i"
> -; with "%.sroa.speculated + 1".
> +; Check IndVarSimplify doesn't replace external use of the induction var
> +; "%inc.i.i" with "%.sroa.speculated + 1" because it is not profitable.
> ;
> ; CHECK-LABEL: @foo(
> -; CHECK: %[[EXIT:.+]] = sub i32 %.sroa.speculated, -1
> +; CHECK: %[[EXIT:.+]] = phi i32 [ %inc.i.i, %for.body650 ]
> ; CHECK: %DB.sroa.9.0.lcssa = phi i32 [ 1, %entry ], [ %[[EXIT]], %loopexit ]
> ;
> define void @foo(i32 %sub.ptr.div.i, i8* %ref.i1174) local_unnamed_addr {
>
> Modified: llvm/trunk/test/Transforms/IndVarSimplify/dont-recompute.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/dont-recompute.ll?rev=345814&r1=345813&r2=345814&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/IndVarSimplify/dont-recompute.ll (original)
> +++ llvm/trunk/test/Transforms/IndVarSimplify/dont-recompute.ll Wed Oct 31 23:47:01 2018
> @@ -123,3 +123,54 @@ for.end:
> tail call void @func(i32 %soft_use)
> ret void
> }
> +
> +; CHECK-LABEL: @test5(
> +define void @test5(i32 %m) 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 ]
> + %add = add i32 %a.05, %m
> + %soft_use = add i32 %add, 123
> +; CHECK: tail call void @func(i32 %soft_use)
> + tail call void @func(i32 %soft_use)
> + %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
> +; CHECK: for.end:
> +; CHECK-NOT: mul i32 %m, 186
> +; CHECK:%add.lcssa = phi i32 [ %add, %for.body ]
> +; CHECK-NEXT: tail call void @func(i32 %add.lcssa)
> + tail call void @func(i32 %add)
> + ret void
> +}
> +
> +; CHECK-LABEL: @test6(
> +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 ]
> + %add = add i32 %a.05, %m
> + %soft_use = add i32 %add, 123
> +; CHECK: store i32 %soft_use, i32* %pidx
> + %pidx = getelementptr i32, i32* %p, i32 %add
> + store i32 %soft_use, i32* %pidx
> + %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
> +; CHECK: for.end:
> +; CHECK-NOT: mul i32 %m, 186
> +; CHECK:%add.lcssa = phi i32 [ %add, %for.body ]
> +; CHECK-NEXT: tail call void @func(i32 %add.lcssa)
> + tail call void @func(i32 %add)
> + ret void
> +}
>
> Modified: llvm/trunk/test/Transforms/IndVarSimplify/lrev-existing-umin.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/lrev-existing-umin.ll?rev=345814&r1=345813&r2=345814&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/IndVarSimplify/lrev-existing-umin.ll (original)
> +++ llvm/trunk/test/Transforms/IndVarSimplify/lrev-existing-umin.ll Wed Oct 31 23:47:01 2018
> @@ -1,5 +1,7 @@
> ; RUN: opt -S -indvars < %s | FileCheck %s
>
> +; Do not rewrite the user outside the loop because we must keep the instruction
> +; inside the loop due to store. Rewrite doesn't give us any profit.
> define void @f(i32 %length.i.88, i32 %length.i, i8* %tmp12, i32 %tmp10, i8* %tmp8) {
> ; CHECK-LABEL: @f(
> not_zero11.preheader:
> @@ -21,6 +23,42 @@ not_zero11:
> %tmp22 = add nuw nsw i32 %v_1, 1
> %tmp23 = icmp slt i32 %tmp22, %tmp14
> br i1 %tmp23, label %not_zero11, label %main.exit.selector
> +
> +main.exit.selector:
> +; CHECK-LABEL: main.exit.selector:
> +; CHECK: %tmp22.lcssa = phi i32 [ %tmp22, %not_zero11 ]
> +; CHECK: %tmp24 = icmp slt i32 %tmp22.lcssa, %length.
> + %tmp24 = icmp slt i32 %tmp22, %length.i
> + br i1 %tmp24, label %not_zero11.postloop, label %leave
> +
> +leave:
> + ret void
> +
> +not_zero11.postloop:
> + ret void
> +}
> +
> +; Rewrite the user outside the loop because there is no hard users inside the loop.
> +define void @f1(i32 %length.i.88, i32 %length.i, i8* %tmp12, i32 %tmp10, i8* %tmp8) {
> +; CHECK-LABEL: @f1(
> +not_zero11.preheader:
> + %tmp13 = icmp ugt i32 %length.i, %length.i.88
> + %tmp14 = select i1 %tmp13, i32 %length.i.88, i32 %length.i
> + %tmp15 = icmp sgt i32 %tmp14, 0
> + br i1 %tmp15, label %not_zero11, label %not_zero11.postloop
> +
> +not_zero11:
> + %v_1 = phi i32 [ %tmp22, %not_zero11 ], [ 0, %not_zero11.preheader ]
> + %tmp16 = zext i32 %v_1 to i64
> + %tmp17 = getelementptr inbounds i8, i8* %tmp8, i64 %tmp16
> + %tmp18 = load i8, i8* %tmp17, align 1
> + %tmp19 = zext i8 %tmp18 to i32
> + %tmp20 = or i32 %tmp19, %tmp10
> + %tmp21 = trunc i32 %tmp20 to i8
> + %addr22 = getelementptr inbounds i8, i8* %tmp12, i64 %tmp16
> + %tmp22 = add nuw nsw i32 %v_1, 1
> + %tmp23 = icmp slt i32 %tmp22, %tmp14
> + br i1 %tmp23, label %not_zero11, label %main.exit.selector
>
> main.exit.selector:
> ; CHECK-LABEL: main.exit.selector:
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: foo.ll
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181102/c0fb9a62/attachment.ksh>
More information about the llvm-commits
mailing list