<div dir="ltr">Thanks guys!<div>I have another small example:</div><div><br></div><div><div>struct A { </div><div> virtual void foo();</div><div>};</div><div><br></div><div>void indirect(A *a, int n) {<br></div><div> for (int i = 0 ; i < n; i++)</div><div> a->foo();</div><div>}</div><div><br></div><div>generates:</div><div><br></div><div><div>define void @_Z8indirectP1Ai(%struct.A* %a, i32 %n) local_unnamed_addr #0 {</div><div>entry:</div><div> %cmp3 = icmp sgt i32 %n, 0</div><div> br i1 %cmp3, label %<a href="http://for.body.lr.ph">for.body.lr.ph</a>, label %for.cond.cleanup</div><div><br></div><div><a href="http://for.body.lr.ph">for.body.lr.ph</a>: ; preds = %entry</div><div> %0 = bitcast %struct.A* %a to void (%struct.A*)***</div><div> br label %for.body</div><div><br></div><div>for.cond.cleanup.loopexit: ; preds = %for.body</div><div> br label %for.cond.cleanup</div><div><br></div><div>for.cond.cleanup: ; preds = %for.cond.cleanup.loopexit, %entry</div><div> ret void</div><div><br></div><div>for.body: ; preds = %for.body, %<a href="http://for.body.lr.ph">for.body.lr.ph</a></div><div> %i.04 = phi i32 [ 0, %<a href="http://for.body.lr.ph">for.body.lr.ph</a> ], [ %inc, %for.body ]</div><div> %vtable = load void (%struct.A*)**, void (%struct.A*)*** %0, align 8, !tbaa !4, !invariant.group !7</div><div> %1 = load void (%struct.A*)*, void (%struct.A*)** %vtable, align 8, !invariant.load !8</div><div> tail call void %1(%struct.A* %a)</div><div> %inc = add nuw nsw i32 %i.04, 1</div><div> %exitcond = icmp eq i32 %inc, %n</div><div> br i1 %exitcond, label %for.cond.cleanup.loopexit, label %for.body</div><div>}</div></div><div><br></div><div><br></div><div>So now `a` is not dereferenceable and I was counting that because we have loop preheader that only executes if the loop executes,</div><div>it will be hoisted. for invariant.group load the isGuaranteedToExecute returns with (!SafetyInfo->HeaderMayThrow).</div><div>The isGuaranteedToTransferExecutionToSuccessor called on the %vtable instruction returns true, </div><div>but few instructions later, the call of %1 may throw. </div><div>Do I understand it correctly, that it is legal to do the hoist because all of the instructions above %vtable does not throw?</div><div>Are there any plans to fix it in the future? The fix doesn't seem hard to write and I can do it, but I am not sure if it won't be too expensive.</div><div><br></div><div>Piotr</div><div><br></div><div>PS: please review the <a href="https://reviews.llvm.org/D31539">https://reviews.llvm.org/D31539</a></div><div> </div></div></div><div class="gmail_extra"><br><div class="gmail_quote">2017-03-31 19:33 GMT+02:00 Daniel Berlin <span dir="ltr"><<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Fri, Mar 31, 2017 at 10:23 AM, Sanjoy Das <span dir="ltr"><<a href="mailto:sanjoy@playingwithpointers.com" target="_blank">sanjoy@playingwithpointers.<wbr>com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Piotr,<br>
<br>
On March 31, 2017 at 9:07:42 AM, Piotr Padlewski<br>
<span>(<a href="mailto:piotr.padlewski@gmail.com" target="_blank">piotr.padlewski@gmail.com</a>) wrote:<br>
> Hi all,<br>
> I have a question about dereferenceable metadata on load instruction. I<br>
> have a patch (<a href="https://reviews.llvm.org/D31539" rel="noreferrer" target="_blank">https://reviews.llvm.org/D315<wbr>39</a>) for LICM that hoists loads<br>
> with !invariant.group.<br>
> The motivation example is devirtualization:<br>
</span>> ...<br>
> [snip]<br>
<span>><br>
> On the other hand, after performing my LICM of !invariant.group load, GVN<br>
> hoists the second load and I am not sure why it is legal then.<br>
<br>
</span>I suspect what's going on is that we first canonicalize the loop to:<br>
<br>
if (precondition) {<br>
do {<br>
vptr = load vtable;<br>
fptr = *vptr;<br>
...<br>
} while (backedge_condition);<br>
}<br>
<br>
after which it is safe to transform the program to (modulo aliasing):<br>
<br>
if (precondition) {<br>
vptr = load vtable;<br>
fptr = *vptr;<br>
do {<br>
...<br>
} while (backedge_condition);<br>
}<br>
<br>
since the we moved a load from a ("strongly") postdominating location.<br>
We know that once we were in the preheader we know we're definitely<br>
going to execute the vptr and fptr loads, so they better be<br>
dereferenceable. In other words, we're "exploiting undefined<br>
behavior" here.<br></blockquote><div><br></div></div></div><div>Yes, this appears to be exactly the case - dominance tells us they must execute at least once in both situations.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="m_-3571807738663522012HOEnZb"><font color="#888888"><br>
-- Sanjoy<br>
</font></span></blockquote></div><br></div></div>
</blockquote></div><br></div>