<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 10/6/21 1:46 PM, Nikita Popov wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF+90c8UQs237hFC91p8S+52tKPOQ-tfHdAZ1F_3-2V8GGKavA@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, Oct 6, 2021 at 10:32
PM Philip Reames <<a
href="mailto:listmail@philipreames.com"
moz-do-not-send="true">listmail@philipreames.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">Er, do you actually have
evidence of this being a problem? I would be <br>
very surprised if calling DT was even *remotely* expensive
here, and I <br>
would strongly prefer the simpler code.<br>
<br>
Philip<br>
</blockquote>
<div><br>
</div>
<div>Yes, this does recover the minor regression that calling
dominates() here introduced.</div>
</div>
</div>
</blockquote>
Fair enough, thanks. And again, sorry for the sharpness of tone on
the original response. That was uncalled for on my part. <br>
<blockquote type="cite"
cite="mid:CAF+90c8UQs237hFC91p8S+52tKPOQ-tfHdAZ1F_3-2V8GGKavA@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
<div>Also, I consider this the simpler code. I didn't like how
D111191 couples implementation details, by having to
hardcode the nodes supported by getDefiningScopeBound() in
the caller as well. Now you can always call it and use a
non-null result.<br>
</div>
</div>
</div>
</blockquote>
<p>Hm, with D111191, I can see your point. Amusingly enough the
helper function is back to the signature I'd originally proposed,
largely with this in mind. I'm not a huge fan of the need for the
double check, but given it has a demonstrated benefit, it seems
called for. <br>
</p>
<p>Anyways, thanks!<br>
</p>
<blockquote type="cite"
cite="mid:CAF+90c8UQs237hFC91p8S+52tKPOQ-tfHdAZ1F_3-2V8GGKavA@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
<div>Regards,</div>
<div>Nikita<br>
</div>
<div> <br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
On 10/6/21 1:14 PM, Nikita Popov via llvm-commits wrote:<br>
> Author: Nikita Popov<br>
> Date: 2021-10-06T22:14:04+02:00<br>
> New Revision: 17c20a6dfb7c89ac4eb13990308b731263345918<br>
><br>
> URL: <a
href="https://github.com/llvm/llvm-project/commit/17c20a6dfb7c89ac4eb13990308b731263345918"
rel="noreferrer" target="_blank" moz-do-not-send="true">https://github.com/llvm/llvm-project/commit/17c20a6dfb7c89ac4eb13990308b731263345918</a><br>
> DIFF: <a
href="https://github.com/llvm/llvm-project/commit/17c20a6dfb7c89ac4eb13990308b731263345918.diff"
rel="noreferrer" target="_blank" moz-do-not-send="true">https://github.com/llvm/llvm-project/commit/17c20a6dfb7c89ac4eb13990308b731263345918.diff</a><br>
><br>
> LOG: [SCEV] Avoid unnecessary domination checks (NFC)<br>
><br>
> When determining the defining scope, avoid repeatedly
querying<br>
> dominationg against the function entry instruction.
This ends up<br>
> begin a very common case that we can handle more
efficiently.<br>
><br>
> Added:<br>
> <br>
><br>
> Modified:<br>
> llvm/include/llvm/Analysis/ScalarEvolution.h<br>
> llvm/lib/Analysis/ScalarEvolution.cpp<br>
><br>
> Removed:<br>
> <br>
><br>
><br>
>
################################################################################<br>
> diff --git
a/llvm/include/llvm/Analysis/ScalarEvolution.h
b/llvm/include/llvm/Analysis/ScalarEvolution.h<br>
> index df1b7c2ad157..a38b1a299163 100644<br>
> --- a/llvm/include/llvm/Analysis/ScalarEvolution.h<br>
> +++ b/llvm/include/llvm/Analysis/ScalarEvolution.h<br>
> @@ -1921,9 +1921,10 @@ class ScalarEvolution {<br>
> SCEV::NoWrapFlags getNoWrapFlagsFromUB(const Value
*V);<br>
> <br>
> /// Return a scope which provides an upper bound on
the defining scope of<br>
> - /// 'S'. Specifically, return the first instruction
in said bounding scope.<br>
> + /// 'S'. Specifically, return the first instruction
in said bounding scope.<br>
> + /// Return nullptr if the scope is trivial (function
entry).<br>
> /// (See scope definition rules associated with
flag discussion above)<br>
> - const Instruction *getDefiningScopeBound(const SCEV
*S);<br>
> + const Instruction
*getNonTrivialDefiningScopeBound(const SCEV *S);<br>
> <br>
> /// Return a scope which provides an upper bound on
the defining scope for<br>
> /// a SCEV with the operands in Ops.<br>
><br>
> diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp
b/llvm/lib/Analysis/ScalarEvolution.cpp<br>
> index b767adac6a59..14f5dd51c406 100644<br>
> --- a/llvm/lib/Analysis/ScalarEvolution.cpp<br>
> +++ b/llvm/lib/Analysis/ScalarEvolution.cpp<br>
> @@ -6585,25 +6585,24 @@ SCEV::NoWrapFlags
ScalarEvolution::getNoWrapFlagsFromUB(const Value *V) {<br>
> return isSCEVExprNeverPoison(BinOp) ? Flags :
SCEV::FlagAnyWrap;<br>
> }<br>
> <br>
> -const Instruction
*ScalarEvolution::getDefiningScopeBound(const SCEV *S) {<br>
> +const Instruction *<br>
> +ScalarEvolution::getNonTrivialDefiningScopeBound(const
SCEV *S) {<br>
> if (auto *AddRec =
dyn_cast<SCEVAddRecExpr>(S))<br>
> return
&*AddRec->getLoop()->getHeader()->begin();<br>
> if (auto *U = dyn_cast<SCEVUnknown>(S))<br>
> if (auto *I =
dyn_cast<Instruction>(U->getValue()))<br>
> return I;<br>
> - // All SCEVs are bound by the entry to F<br>
> - return &*F.getEntryBlock().begin();<br>
> + return nullptr;<br>
> }<br>
> <br>
> const Instruction *<br>
>
ScalarEvolution::getDefiningScopeBound(ArrayRef<const
SCEV *> Ops) {<br>
> - const Instruction *Bound =
&*F.getEntryBlock().begin();<br>
> - for (auto *S : Ops) {<br>
> - auto *DefI = getDefiningScopeBound(S);<br>
> - if (DT.dominates(Bound, DefI))<br>
> - Bound = DefI;<br>
> - }<br>
> - return Bound;<br>
> + const Instruction *Bound = nullptr;<br>
> + for (auto *S : Ops)<br>
> + if (auto *DefI =
getNonTrivialDefiningScopeBound(S))<br>
> + if (!Bound || DT.dominates(Bound, DefI))<br>
> + Bound = DefI;<br>
> + return Bound ? Bound :
&*F.getEntryBlock().begin();<br>
> }<br>
> <br>
> <br>
><br>
><br>
> <br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">llvm-commits@lists.llvm.org</a><br>
> <a
href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"
rel="noreferrer" target="_blank" moz-do-not-send="true">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
</div>
</blockquote>
</body>
</html>