<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2017-04-05 18:30 GMT+02:00 Sanjoy Das <span dir="ltr"><<a href="mailto:sanjoy@playingwithpointers.com" target="_blank">sanjoy@playingwithpointers.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Piotr,<br>
<span class="gmail-"><br>
On Mon, Apr 3, 2017 at 2:01 PM, Piotr Padlewski<br>
<<a href="mailto:piotr.padlewski@gmail.com">piotr.padlewski@gmail.com</a>> wrote:<br>
>> I don't see any way to model it right now in LLVM, but I see a one simple<br>
>> solution:<br>
>>  add extra information to the metadata nodes indicating that this property<br>
>> is non-local:<br>
>><br>
>> %0 = load... %p, !invariant.group !1, !dereferenceable !2<br>
>> %1 = load ... %0, !invariant.load !0, !dereferenceable !2<br>
>><br>
>> !0 = !{!"GlobalProperty"}<br>
>> !1 = !{!"MyType", !"GlobalProperty"}<br>
>> !2 = !{i64 8, !"GlobalProperty}<br>
>><br>
>> With that we would strip only the metadata not containing this<br>
>> information.<br>
>><br>
>> For devirtualization it would make sense with invariant.load,<br>
>> invariant.group and dereferenceable.<br>
>><br>
>> What do you think about this idea?<br>
<br>
</span>I'm sorry for being *this* annoying, but I'm not on board with this.  :)<br>
<br>
I think this will run into the same dead-code-can-affect-behavior<br>
issue discussed in <a href="https://reviews.llvm.org/D20116" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D20116</a>.<br>
<br>
Specifically, if your program is:<br>
<br>
if (false) {<br>
  ptr = load i8*, i8** %ptrptr, !dereferenceable !{i64 8, !"GlobalProperty}<br>
  // ptr is not actually dereferenceable, even the load above has UB<br>
  // (since the metadata is "wrong"), but it is never executed so all is well.<br>
  int val = *ptr;<br>
}<br>
<br>
then you could transform it to<br>
<br>
ptr = load i8*, i8** %ptrptr, !dereferenceable !{i64 8, !"GlobalProperty}<br>
// ptr is not actually dereferenceable<br>
int val = *ptr;<br>
if (false) {<br>
}<br>
<br>
and you'd have gone from a program with no UB to one with UB.<br>
<span class="gmail-HOEnZb"><font color="#888888"><br>
-- Sanjoy<br>
</font></span></blockquote></div>I disagree, I find it different than the patch you mentioned. We don't have any problems with code like this:</div><div class="gmail_extra"><br></div><div class="gmail_extra">ptr = load i8*, i8** %ptrptr, !dereferenceable !{i64 8}<br></div><div class="gmail_extra">if (false) {<br><br>  // ptr is not actually dereferenceable, even the load above has UB<br>  // (since the metadata is "wrong"), but it is never executed so all is well.<br>  int val = *ptr;<br>}<br></div><div class="gmail_extra"><br></div><div class="gmail_extra">because we rely on the information provided by someone else that ptr is dereferenceable, so we can transform it to</div><div class="gmail_extra"><br></div><div class="gmail_extra">ptr = load i8*, i8** %ptrptr, !dereferenceable !{i64 8}<br>// ptr is not actually dereferenceable<br>int val = *ptr;<br>if (false) {<br>}<br></div><div class="gmail_extra"><br></div><div class="gmail_extra">And this is exactly how we treat any other UB.</div><div class="gmail_extra">It is also important to mention that in order to perform optimization you have mentioned we need to know that %ptrptr is also dereferenceable.</div><div class="gmail_extra">The vptr and vtable properties are non local, so I don't see how hoisting vptr load could be dangerous:</div><div class="gmail_extra"><br></div><div class="gmail_extra">void foo(A& a) {</div><div class="gmail_extra">  if (false) </div><div class="gmail_extra">    a.foo();</div><div class="gmail_extra">}</div><div class="gmail_extra"><br></div><div class="gmail_extra">will be:</div><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_extra">void foo(A* dereferenceable(8) %a) {</div><div class="gmail_extra">  if (false)</div><div class="gmail_extra"><div class="gmail_extra">    %vptr = load %a, !dereferenceable !{VtableSize, "GlobalProperty"}, !invariant.group !0</div><div class="gmail_extra">    %vfunction = load %vptr, !invariant.load !0</div></div><div class="gmail_extra">    call %vfunction();</div><div class="gmail_extra">}</div><div><br></div></div><div class="gmail_extra">and after hoisting:</div><div class="gmail_extra"><br></div><div class="gmail_extra">void foo(A* dereferenceable(8) %a) {</div><div class="gmail_extra">  %vptr = load %a, !dereferenceable !{VtableSize, "GlobalProperty"}, !invariant.group !{"GlobalProperty"}</div><div class="gmail_extra">  %vfunction = load %vptr, !invariant.load !{"GlobalProperty"}</div><div class="gmail_extra">  if (false)</div><div class="gmail_extra">    call %vfunction();</div><div class="gmail_extra">}</div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div></div>