<div dir="ltr"><div dir="ltr">Hello Philip,<br><br>I agree that the current (UB) semantics is simpler 

in terms of interprocedural analysis, 

.<br><br>My concern was that the semantics of nonnull was too strong (too undefined) for optimizations that attach nonnull, such as the InstCombine optimization. It seems there is a class of such transformations, such as deriving nonnull from inttoptr(or x, 1).</div><div dir="ltr"><div></div><div>If nonnull raises UB, attaching nonnull from this is problematic, as these operations don't raise UB but propagates poison.</div><div><br></div><div>Or this can be supported by adding 'nonnull_or_poison', </div><div>Would this make sense? It wouldn't touch the existing semantics of nonnull.</div><div><div>Also, if an argument is not_poison and nonnull_or_poison, they can be merged into nonnull.</div><div></div></div><div><br></div><div>Juneyoung Lee</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Feb 19, 2020 at 6:49 AM 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">I'm really not sure I agree there's a problem with the current semantics <br>
at all.<br>
<br>
The root of your argument appears to be that passing poison to a <br>
function argument is "harmless" if that argument is unused in the <br>
callee.  This seems reasonable to me, but it brings with it a <br>
restriction on any IPO pass which is non-obvious.  It essentially <br>
requires that the IPA argument inference framework prove both that a <br>
property holds, and that if that property didn't hold UB would be <br>
triggered.  (I think this is similar in spirit to the "used" discussion <br>
downthread.)   In general, it is much easier to prove the former <br>
property than the later.<br>
<br>
I would simply state that per the current semantics, any IPO/IPA <br>
transform which doesn't do so is buggy.  That's not pretty, but it's <br>
valid and I believe matches (most of?) the current implementation.<br>
<br>
Conversely, the power for the frontend to annotate such facts on the <br>
function boundary is one of the key advantages higher level semantics <br>
give over inference.  I would be very reluctant to give that up.<br>
<br>
To be clear, I'm completely open to proposals to change the current <br>
semantics for ease of implementation on the IPO/IPA side. I just think <br>
it's important to distinguish between "current semantics are wrong" and <br>
"current semantics are inconvenient".<br>
<br>
Philip<br>
<br>
On 2/17/20 5:45 PM, Juneyoung Lee wrote:<br>
> Hello all,<br>
><br>
> LangRef says it is undefined behavior to pass null to a nonnull <br>
> argument (`call f(nonnull null);`), but the semantics is too strong <br>
> for a few existing optimizations.<br>
> To support these, we can relax the semantics so `f(nonnull null)` is <br>
> equivalent to `f(poison)`, but (A) it again blocks another set of <br>
> optimizations, and (B) this makes the semantics of nonnull deviate <br>
> from other attributes like dereferenceable.<br>
> I found that there was a similar discussion about this issue in the <br>
> past as well, but seems it is not settled yet.<br>
> What should the semantics of nonnull be?<br>
> I listed a few optimizations that are relevant with this issue.<br>
><br>
><br>
> 1. Propagating nonnull attribute to callee's arg <br>
> (<a href="https://godbolt.org/z/-cVsVP" rel="noreferrer" target="_blank">https://godbolt.org/z/-cVsVP</a> )<br>
><br>
> g(i8* ptr) {<br>
> f(nonnull ptr);<br>
> }<br>
> =><br>
> g(i8* nonnull ptr) {<br>
> f(nonnull ptr);<br>
> }<br>
><br>
> This is correct if f(nonnull null) is UB. If ptr == null, f(nonnull <br>
> null) should have raised UB, so ptr shouldn't be null.<br>
> However, this transformation is incorrect if f(nonnull null) is <br>
> equivalent to f(poison).<br>
> If f was an empty function, f(nonnull null) never raises UB regardless <br>
> of ptr. So we can't guarantee ptr != null at other uses of ptr.<br>
><br>
><br>
> 2. InstCombine (<a href="https://godbolt.org/z/HDQ7rD" rel="noreferrer" target="_blank">https://godbolt.org/z/HDQ7rD</a> ):<br>
><br>
> %ptr_inb = gep inbounds %any_ptr, 1<br>
> f(%ptr_inb)<br>
> =><br>
> %ptr_inb = .. (identical)<br>
> f(nonnull %ptr_inb)<br>
><br>
> This optimization is incorrect if `f(nonnull null)` is UB. The reason <br>
> is as follows.<br>
> If `gep inbounds %any_ptr, 1` yields poison, the source is `f(poison)` <br>
> whereas the optimized one is `f(nonnull poison)`.<br>
> `f(nonnull poison)` should be UB because `f(nonnull null)` is UB. So, <br>
> the transformation introduced UB.<br>
> This optimization is correct if both `f(nonnull null)` and `f(nonnull <br>
> poison)` are equivalent to `f(poison)`.<br>
><br>
><br>
> 3. <a href="https://reviews.llvm.org/D69477" rel="noreferrer" target="_blank">https://reviews.llvm.org/D69477</a><br>
><br>
> f(nonnull ptr);<br>
> use(ptr);<br>
> =><br>
> llvm.assume(ptr != null);<br>
> use(ptr);<br>
> f(nonnull ptr);<br>
><br>
> If f(nonnull null) is f(poison), this is incorrect. If ptr was null, <br>
> the added llvm.assume(ptr != null) raises UB whereas the source may <br>
> not raise UB at all. (e.g. assume that f() was an empty function)<br>
> If f(nonnull null) is UB, this is correct.<br>
><br>
><br>
> 4. Dead argument elimination (from <a href="https://reviews.llvm.org/D70749" rel="noreferrer" target="_blank">https://reviews.llvm.org/D70749</a> )<br>
><br>
> f(nonnull ptr); // f’s argument is dead<br>
> =><br>
> f(nonnull undef);<br>
><br>
> This is incorrect if f(nonnull null) is UB. To make this correct, <br>
> nonnull should be dropped. This becomes harder to fix if nonnull was <br>
> attached at the signature of a function (not at the callee site).<br>
> It is correct if f(nonnull null) is f(poison).<br>
><br>
> Actually the D70749's thread had an end-to-end miscompilation example <br>
> due to the interaction between this DAE and the optimization 3 <br>
> (insertion of llvm.assume).<br>
><br>
> Thanks,<br>
> Juneyoung Lee<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><br></div><font size="1">Juneyoung Lee</font><div><font size="1">Software Foundation Lab, Seoul National University</font></div></div></div></div>