<div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">My point is that SimplifyCall is allowed to simplify one side effecting call to another side effecting call.<br clear="all"></blockquote><div><br></div><div>It also may simplify side effecting call to something that has no side effect, if the side effect is actually absent. It takes place for the code: </div><div><br></div><div>define float @f_unused_precise() #0 {<br> %result = call float @llvm.experimental.constrained.fadd.f32(float 1.0, float 1.0, metadata !"round.upward", metadata !"fpexcept.strict") #0<br> ret float 1.0<br>}<br></div><div><br></div><div>In it experimental.constrained.fadd has side effect, because exception behavior is strict, but SimplifyCall can reveal that actually side effect is absent and simplifies the call to the constant 2.0. As the result of the call is not used the net result is removal of the call.</div><div><br></div><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature">Thanks,<br>--Serge<br></div></div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, May 5, 2022 at 11:17 PM Philip Reames <<a href="mailto:listmail@philipreames.com">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">
<div>
<p><br>
</p>
<div>On 5/5/22 09:14, Serge Pavlov wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This looks incorrect to
me. SimplifyCall returns a Value* for a<br>
simplified expression and you're interpreting that as a
boolean meaning<br>
"delete call".<br>
</blockquote>
<div><br>
</div>
<div>In this case SimplifyCall is called for a call, which is
not used and was not removed only because it has declared
side effect. Non-null replacement returned by SimplifyCall
is used as an indicator that the call may be simplified, so
its side effect is absent or may be ignored. Such unused
calls appear as a result of other transformations, they are
not removed due to the side effect and produce useless
instructions in resulting code. <br>
</div>
</div>
</div>
</blockquote>
<p>My point is that SimplifyCall is allowed to simplify one side
effecting call to another side effecting call. As in, you can't
make assumptions about *which* simplifications SimplifyCall has
made. If it returns a nonnull result, you must use it (or leave
the instruction unmodified). <br>
</p>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<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">I would expect this to be
in isInstructionTriviallyDead anyways.<br>
</blockquote>
<div><br>
</div>
<div>It would be nice but the concern is that simplifying such
calls is expensive and isInstructionTriviallyDead is called
often enough, so negative impact on compile time is
unavoidable. There are many transformations, not only
constant folding and their number will increase, so putting
all of them into isInstructionTriviallyDead does not look
like a good design.</div>
<div><br>
</div>
<div>Probably it is not a perfect solution, but it seems that
we have no other solution at the moment. This problem was
discussed in <a href="https://reviews.llvm.org/D118426" rel="noreferrer" target="_blank">https://reviews.llvm.org/D118426</a> and
in some previous patch reviews. If you have any ideas you
could continue that discussion.</div>
<div> </div>
<div>
<div dir="ltr">Thanks,<br>
--Serge<br>
</div>
</div>
<br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, May 5, 2022 at 10:00
PM Philip Reames <<a href="mailto:listmail@philipreames.com" target="_blank">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"><br>
On 5/4/22 22:03, Serge Pavlov via llvm-commits wrote:<br>
> Author: Serge Pavlov<br>
> Date: 2022-05-05T12:02:42+07:00<br>
> New Revision: 83914ee96fc2d828e1cfb8913f5d156d39150e2c<br>
><br>
> URL: <a href="https://github.com/llvm/llvm-project/commit/83914ee96fc2d828e1cfb8913f5d156d39150e2c" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/83914ee96fc2d828e1cfb8913f5d156d39150e2c</a><br>
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/83914ee96fc2d828e1cfb8913f5d156d39150e2c.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/83914ee96fc2d828e1cfb8913f5d156d39150e2c.diff</a><br>
><br>
> LOG: [InstCombine] Remove side effect of replaced
constrained intrinsics<br>
><br>
> If a constrained intrinsic call was replaced by some
value, it was not<br>
> removed in some cases. The dangling instruction
resulted in useless<br>
> instructions executed in runtime. It happened because
constrained<br>
> intrinsics usually have side effect, it is used to
model the interaction<br>
> with floating-point environment. In some cases it is
correct behavior<br>
> but often the side effect is actually absent or can be
ignored.<br>
><br>
> This change adds specific treatment of constrained
intrinsics so that<br>
> their side effect can be removed if it actually
absents.<br>
><br>
> Differential Revision: <a href="https://reviews.llvm.org/D118426" rel="noreferrer" target="_blank">https://reviews.llvm.org/D118426</a><br>
><br>
> Added:<br>
> llvm/test/Transforms/InstCombine/constrained.ll<br>
><br>
> Modified:<br>
> llvm/include/llvm/Analysis/InstructionSimplify.h<br>
>
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>
><br>
> Removed:<br>
> <br>
><br>
><br>
>
################################################################################<br>
> diff --git
a/llvm/include/llvm/Analysis/InstructionSimplify.h
b/llvm/include/llvm/Analysis/InstructionSimplify.h<br>
> index 8f6ed3a6a1928..612a73551f720 100644<br>
> --- a/llvm/include/llvm/Analysis/InstructionSimplify.h<br>
> +++ b/llvm/include/llvm/Analysis/InstructionSimplify.h<br>
> @@ -299,6 +299,10 @@ Value *SimplifyBinOp(unsigned
Opcode, Value *LHS, Value *RHS, FastMathFlags FMF,<br>
> const SimplifyQuery &Q);<br>
> <br>
> /// Given a callsite, fold the result or return null.<br>
> +///<br>
> +/// \note A call with declared side effect may be
simplified into a value<br>
> +/// without such. It happens if simplification code
deduces that side effect<br>
> +/// is actually absent.<br>
> Value *SimplifyCall(CallBase *Call, const
SimplifyQuery &Q);<br>
> <br>
> /// Given an operand for a Freeze, see if we can fold
the result.<br>
><br>
> diff --git
a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>
> index c96919caee2b1..6b2ab24b4b746 100644<br>
> ---
a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>
> +++
b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>
> @@ -1237,6 +1237,16 @@ Instruction
*InstCombinerImpl::visitCallInst(CallInst &CI) {<br>
> return NewCall;<br>
> }<br>
> <br>
> + // Unused constrained FP intrinsic calls may have
declared side effect, which<br>
> + // actually absent. If SimplifyCall returns a
replacement for such call,<br>
> + // assume side effect is absent and the call may be
removed.<br>
> + if (CI.use_empty() &&
isa<ConstrainedFPIntrinsic>(CI)) {<br>
> + if (SimplifyCall(&CI,
SQ.getWithInstruction(&CI))) {<br>
> + eraseInstFromFunction(CI);<br>
> + return nullptr;<br>
> + }<br>
> + }<br>
> +<br>
<br>
This looks incorrect to me. SimplifyCall returns a Value*
for a <br>
simplified expression and you're interpreting that as a
boolean meaning <br>
"delete call".<br>
<br>
I would expect this to be in isInstructionTriviallyDead
anyways.<br>
<br>
Please revert unless I'm misreading.<br>
<br>
> Intrinsic::ID IID = II->getIntrinsicID();<br>
> switch (IID) {<br>
> case Intrinsic::objectsize:<br>
><br>
> diff --git
a/llvm/test/Transforms/InstCombine/constrained.ll
b/llvm/test/Transforms/InstCombine/constrained.ll<br>
> new file mode 100644<br>
> index 0000000000000..b5ef71e6edfba<br>
> --- /dev/null<br>
> +++ b/llvm/test/Transforms/InstCombine/constrained.ll<br>
> @@ -0,0 +1,125 @@<br>
> +; NOTE: Assertions have been autogenerated by
utils/update_test_checks.py<br>
> +; RUN: opt -S -instcombine %s | FileCheck %s<br>
> +<br>
> +; Treatment of operation with unused result.<br>
> +<br>
> +; If operation does not raise exceptions, it may be
removed even in strict mode.<br>
> +define float @f_unused_precise() #0 {<br>
> +; CHECK-LABEL: @f_unused_precise(<br>
> +; CHECK-NEXT: entry:<br>
> +; CHECK-NEXT: ret float 1.000000e+00<br>
> +;<br>
> +entry:<br>
> + %result = call float
@llvm.experimental.constrained.fadd.f32(float 1.0, float
1.0, metadata !"round.upward", metadata !"fpexcept.strict")
#0<br>
> + ret float 1.0<br>
> +}<br>
> +<br>
> +; If operation raises exceptions, it cannot be removed
in strict mode.<br>
> +define float @f_unused_strict() #0 {<br>
> +; CHECK-LABEL: @f_unused_strict(<br>
> +; CHECK-NEXT: entry:<br>
> +; CHECK-NEXT: [[RESULT:%.*]] = call float
@llvm.experimental.constrained.fdiv.f32(float 1.000000e+00,
float 3.000000e+00, metadata !"round.tonearest", metadata
!"fpexcept.strict") #[[ATTR0:[0-9]+]]<br>
> +; CHECK-NEXT: ret float 1.000000e+00<br>
> +;<br>
> +entry:<br>
> + %result = call float
@llvm.experimental.constrained.fdiv.f32(float 1.0, float
3.0, metadata !"round.tonearest", metadata
!"fpexcept.strict") #0<br>
> + ret float 1.0<br>
> +}<br>
> +<br>
> +; If operation raises exceptions, it can be removed in
non-strict mode.<br>
> +define float @f_unused_ignore() #0 {<br>
> +; CHECK-LABEL: @f_unused_ignore(<br>
> +; CHECK-NEXT: entry:<br>
> +; CHECK-NEXT: ret float 1.000000e+00<br>
> +;<br>
> +entry:<br>
> + %result = call float
@llvm.experimental.constrained.fdiv.f32(float 1.0, float
3.0, metadata !"round.towardzero", metadata
!"fpexcept.ignore") #0<br>
> + ret float 1.0<br>
> +}<br>
> +<br>
> +; If operation raises exceptions, it can be removed in
non-strict mode even if rounding mode is dynamic.<br>
> +define float @f_unused_dynamic_ignore() #0 {<br>
> +; CHECK-LABEL: @f_unused_dynamic_ignore(<br>
> +; CHECK-NEXT: entry:<br>
> +; CHECK-NEXT: ret float 1.000000e+00<br>
> +;<br>
> +entry:<br>
> + %result = call float
@llvm.experimental.constrained.fdiv.f32(float 1.0, float
3.0, metadata !"round.dynamic", metadata !"fpexcept.ignore")
#0<br>
> + ret float 1.0<br>
> +}<br>
> +<br>
> +; If operation raises exceptions, it can be removed in
"maytrap" mode.<br>
> +define float @f_unused_maytrap() #0 {<br>
> +; CHECK-LABEL: @f_unused_maytrap(<br>
> +; CHECK-NEXT: entry:<br>
> +; CHECK-NEXT: ret float 1.000000e+00<br>
> +;<br>
> +entry:<br>
> + %result = call float
@llvm.experimental.constrained.fdiv.f32(float 1.0, float
3.0, metadata !"round.tonearest", metadata
!"fpexcept.maytrap") #0<br>
> + ret float 1.0<br>
> +}<br>
> +<br>
> +; Constant evaluation.<br>
> +<br>
> +; If operation does not raise exceptions, it may be
folded even in strict mode.<br>
> +define float @f_eval_precise() #0 {<br>
> +; CHECK-LABEL: @f_eval_precise(<br>
> +; CHECK-NEXT: entry:<br>
> +; CHECK-NEXT: ret float 2.000000e+00<br>
> +;<br>
> +entry:<br>
> + %result = call float
@llvm.experimental.constrained.fadd.f32(float 1.0, float
1.0, metadata !"round.upward", metadata !"fpexcept.strict")
#0<br>
> + ret float %result<br>
> +}<br>
> +<br>
> +; If operation raises exceptions, it cannot be folded
in strict mode.<br>
> +define float @f_eval_strict() #0 {<br>
> +; CHECK-LABEL: @f_eval_strict(<br>
> +; CHECK-NEXT: entry:<br>
> +; CHECK-NEXT: [[RESULT:%.*]] = call float
@llvm.experimental.constrained.fdiv.f32(float 1.000000e+00,
float 3.000000e+00, metadata !"round.upward", metadata
!"fpexcept.strict") #[[ATTR0]]<br>
> +; CHECK-NEXT: ret float [[RESULT]]<br>
> +;<br>
> +entry:<br>
> + %result = call float
@llvm.experimental.constrained.fdiv.f32(float 1.0, float
3.0, metadata !"round.upward", metadata !"fpexcept.strict")
#0<br>
> + ret float %result<br>
> +}<br>
> +<br>
> +; If operation raises exceptions, it can be folded in
non-strict mode.<br>
> +define float @f_eval_ignore() #0 {<br>
> +; CHECK-LABEL: @f_eval_ignore(<br>
> +; CHECK-NEXT: entry:<br>
> +; CHECK-NEXT: ret float 0x3FD5555540000000<br>
> +;<br>
> +entry:<br>
> + %result = call float
@llvm.experimental.constrained.fdiv.f32(float 1.0, float
3.0, metadata !"round.downward", metadata
!"fpexcept.ignore") #0<br>
> + ret float %result<br>
> +}<br>
> +<br>
> +; if result is imprecise, it cannot be folded if
rounding mode is dynamic.<br>
> +define float @f_eval_dynamic_ignore() #0 {<br>
> +; CHECK-LABEL: @f_eval_dynamic_ignore(<br>
> +; CHECK-NEXT: entry:<br>
> +; CHECK-NEXT: [[RESULT:%.*]] = call float
@llvm.experimental.constrained.fdiv.f32(float 1.000000e+00,
float 3.000000e+00, metadata !"round.dynamic", metadata
!"fpexcept.ignore") #[[ATTR0]]<br>
> +; CHECK-NEXT: ret float [[RESULT]]<br>
> +;<br>
> +entry:<br>
> + %result = call float
@llvm.experimental.constrained.fdiv.f32(float 1.0, float
3.0, metadata !"round.dynamic", metadata !"fpexcept.ignore")
#0<br>
> + ret float %result<br>
> +}<br>
> +<br>
> +; If result is imprecise and rounding mode is not
dynamic, operation can be folded in "maytrap" mode.<br>
> +define float @f_eval_maytrap() #0 {<br>
> +; CHECK-LABEL: @f_eval_maytrap(<br>
> +; CHECK-NEXT: entry:<br>
> +; CHECK-NEXT: ret float 0x3FD5555560000000<br>
> +;<br>
> +entry:<br>
> + %result = call float
@llvm.experimental.constrained.fdiv.f32(float 1.0, float
3.0, metadata !"round.tonearest", metadata
!"fpexcept.maytrap") #0<br>
> + ret float %result<br>
> +}<br>
> +<br>
> +<br>
> +declare float
@llvm.experimental.constrained.fadd.f32(float, float,
metadata, metadata)<br>
> +declare float
@llvm.experimental.constrained.fdiv.f32(float, float,
metadata, metadata)<br>
> +<br>
> +attributes #0 = { strictfp }<br>
><br>
><br>
> <br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
</div>
</blockquote>
</div>
</blockquote></div>