<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2017-03-31 23:20 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>
<br>
On March 31, 2017 at 1:07:12 PM, Piotr Padlewski<br>
(<a href="mailto:piotr.padlewski@gmail.com">piotr.padlewski@gmail.com</a>) wrote:<br>
> [snip]<br>
<span class="gmail-">> Do I understand it correctly, that it is legal to do the hoist because all<br>
> of the instructions above %vtable does not throw?<br>
<br>
</span>Yes, I think you're right.  HeaderMayThrow is a conservative<br>
approximation, and the conservativeness is biting us here.<br>
<span class="gmail-"><br>
> Are there any plans to fix it in the future? The fix doesn't seem hard to<br>
<br>
</span>Not to my knowledge.<br>
<span class="gmail-"><br>
> write and I can do it, but I am not sure if it won't be too expensive.<br>
<br>
</span>Maybe we can do it (relatively) cheaply:<br>
<br>
 - Remember the first throwing instruction in the header, instead of a<br>
   boolean, in LoopSafetyInfo<br>
<br>
 - In hoistRegion, remember if you've seen the first throwing<br>
   instruction yet<br>
<br>
 - Pass the above as a boolean parameter to isGuaranteedToExecute, and<br>
   instead of<br>
     if (Inst.getParent() == CurLoop->getHeader())<br>
       return !SafetyInfo->HeaderMayThrow;<br>
   do something like<br>
     if (Inst.getParent() == CurLoop->getHeader())<br>
       return IsBeforeThrowingInst;<br>
<br>
-- Sanjoy<br>
</blockquote></div>I was thinking about something very similar and it seems to be a good approach to me because it has much lower complexity.</div><div class="gmail_extra"><br></div><div class="gmail_extra">In the review Sanjoy correctly spoted that I am not discarding invariant.group metadata when hoisting, which is incorrect in general.</div><div class="gmail_extra">This generates a big problem to devirtualization, because discarding metadata when hoisting vtable load might be worse than not hoisting when</div><div class="gmail_extra">there might be another load/store with !invariant.group dominating it (e.g. after inlining).</div><div class="gmail_extra"><br></div><div class="gmail_extra">I don't see any way to model it right now in LLVM, but I see a one simple solution:</div><div class="gmail_extra"> add extra information to the metadata nodes indicating that this property is non-local:</div><div class="gmail_extra"><br></div><div class="gmail_extra">%0 = load... %p, !invariant.group !1, !dereferenceable !2</div><div class="gmail_extra"><span style="font-size:12.8px">%1 = load ... %0, !invariant.load !0, !dereferenceable !2</span><br></div><div class="gmail_extra"><span style="font-size:12.8px"><br></span></div><div class="gmail_extra"><span style="font-size:12.8px">!0 = !{!"GlobalProperty"}</span></div><div class="gmail_extra"><span style="font-size:12.8px">!1 = !{!"MyType", !"GlobalProperty"}</span></div><div class="gmail_extra"><span style="font-size:12.8px">!2 = !{i64 8, !"GlobalProperty}</span></div><div class="gmail_extra"><span style="font-size:12.8px"><br></span></div><div class="gmail_extra"><span style="font-size:12.8px">With that we would strip only the metadata not containing this information.</span></div><div class="gmail_extra"><br></div><div class="gmail_extra">For devirtualization it would make sense with invariant.load, invariant.group and dereferenceable.</div><div class="gmail_extra"><br></div><div class="gmail_extra">What do you think about this idea? </div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra"><br></div></div>