From: Alina Sbirlea Date: Fri, 12 Apr 2019 18:48:46 +0000 (+0000) Subject: [MemorySSA] Small fix for the clobber limit. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=509313a40193ec5baa4c541ad6bb93e97bdc0ce0;p=llvm [MemorySSA] Small fix for the clobber limit. Summary: After introducing the limit for clobber walking, `walkToPhiOrClobber` would assert that the limit is at least 1 on entry. The test included triggered that assert. The callsite in `tryOptimizePhi` making the calls to `walkToPhiOrClobber` is structured like this: ``` while (true) { if (getBlockingAccess()) { // calls walkToPhiOrClobber } for (...) { walkToPhiOrClobber(); } } ``` The cleanest fix is to check if the limit was reached inside `walkToPhiOrClobber`, and give an allowence of 1. This approach not make any alias() calls (no calls to instructionClobbersQuery), so the performance condition is enforced. The limit is set back to 0 if not used, as this provides info on the fact that we stopped before reaching a true clobber. Reviewers: george.burgess.iv Subscribers: jlebar, Prazek, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D60479 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@358303 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Analysis/MemorySSA.cpp b/lib/Analysis/MemorySSA.cpp index a8100312f21..cd6235de685 100644 --- a/lib/Analysis/MemorySSA.cpp +++ b/lib/Analysis/MemorySSA.cpp @@ -544,10 +544,15 @@ template class ClobberWalker { const MemoryAccess *SkipStopAt = nullptr) const { assert(!isa(Desc.Last) && "Uses don't exist in my world"); assert(UpwardWalkLimit && "Need a valid walk limit"); - // This will not do any alias() calls. It returns in the first iteration in - // the loop below. - if (*UpwardWalkLimit == 0) - (*UpwardWalkLimit)++; + bool LimitAlreadyReached = false; + // (*UpwardWalkLimit) may be 0 here, due to the loop in tryOptimizePhi. Set + // it to 1. This will not do any alias() calls. It either returns in the + // first iteration in the loop below, or is set back to 0 if all def chains + // are free of MemoryDefs. + if (!*UpwardWalkLimit) { + *UpwardWalkLimit = 1; + LimitAlreadyReached = true; + } for (MemoryAccess *Current : def_chain(Desc.Last)) { Desc.Last = Current; @@ -568,6 +573,9 @@ template class ClobberWalker { } } + if (LimitAlreadyReached) + *UpwardWalkLimit = 0; + assert(isa(Desc.Last) && "Ended at a non-clobber that's not a phi?"); return {Desc.Last, false, MayAlias}; diff --git a/test/Analysis/MemorySSA/reduce_clobber_limit.ll b/test/Analysis/MemorySSA/reduce_clobber_limit.ll new file mode 100644 index 00000000000..79c07c1bdfd --- /dev/null +++ b/test/Analysis/MemorySSA/reduce_clobber_limit.ll @@ -0,0 +1,131 @@ +; RUN: opt -S -memoryssa %s | FileCheck %s +; REQUIRES: asserts +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +; CHECK-LABEL: @func() +; Function Attrs: noinline +define dso_local void @func() unnamed_addr #0 align 2 { +entry: + %NoFinalize.addr = alloca i8, align 1 + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + %call8 = call zeroext i1 @foo() + br i1 %call8, label %if.then9, label %while.cond + +if.then9: ; preds = %entry + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + br label %while.cond + +while.cond: ; preds = %cleanup, %if.then9, %entry + %call34 = call zeroext i1 @foo() + call void @blah() + br i1 %call34, label %while.body, label %while.end + +while.body: ; preds = %while.cond + %call35 = call zeroext i1 @foo() + br i1 %call35, label %if.end37, label %if.then36 + +if.then36: ; preds = %while.body + store i32 2, i32* undef, align 4 + br label %cleanup + +if.end37: ; preds = %while.body + %call38 = call zeroext i1 @foo() + br i1 %call38, label %if.end46, label %land.lhs.true + +land.lhs.true: ; preds = %if.end37 + call void @blah() + %call41 = call zeroext i1 @foo() + br i1 %call41, label %if.then42, label %if.end46 + +if.then42: ; preds = %land.lhs.true + call void @blah() + br label %if.end46 + +if.end46: ; preds = %if.then42, %land.lhs.true, %if.end37 + call void @blah() + call void @blah() + call void @blah() + call void @blah() + br label %cleanup + +cleanup: ; preds = %if.end46, %if.then36 + call void @blah() + br label %while.cond + +while.end: ; preds = %while.cond + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + %call93 = call zeroext i1 @foo() + br i1 %call93, label %if.end120, label %if.then94 + +if.then94: ; preds = %while.end + store i32 0, i32* undef, align 4 + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + br label %for.cond + +for.cond: ; preds = %for.body, %if.then94 + br i1 undef, label %for.body, label %if.end120 + +for.body: ; preds = %for.cond + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + call void @blah() + br label %for.cond + +if.end120: ; preds = %for.cond, %while.end + %val = load i8, i8* %NoFinalize.addr, align 1 + ret void +} + +; Function Attrs: noinline +declare hidden void @blah() unnamed_addr #0 align 2 + +; Function Attrs: noinline +declare hidden i1 @foo() local_unnamed_addr #0 align 2 + +attributes #0 = { noinline } +