<div dir="ltr">Hi folks,<div>I would like to ask for some help with handling invariant.group metadata for pointer comparison. Currently, we have a problem with devirtualization</div><div>of this code:</div><div><br></div><div><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="monospace, monospace"><span style="color:rgb(51,51,153);font-weight:bold">void</span> <span style="color:rgb(0,102,187);font-weight:bold">compare</span>() {
A* a = <span style="color:rgb(0,136,0);font-weight:bold">new</span> A;
a->foo();
A* b = <span style="color:rgb(0,136,0);font-weight:bold">new</span>(a) B;
<span style="color:rgb(0,136,0);font-weight:bold">if</span> (a == b) {
b->foo();
}
}</font></pre></div><div><br></div><div>Now because it is legal to replace b with an in the branch the vtable load will use old pointer operand with !invariant.group, which will lead to devirtualization to A::foo().</div><div>I tried to fix that in the frontend by introducing barriers for dynamic objects comparison, like:</div><div><br></div><div>if (barrier(a) == barrier(b))</div><div><br></div><div>Unfortunately, it didn't work out for all of the cases. Example by Hubert Tong</div><div><br></div><div><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="monospace, monospace"><span style="color:rgb(0,136,0);font-weight:bold">struct</span> A {
<span style="color:rgb(0,136,0);font-weight:bold">virtual</span> <span style="color:rgb(51,51,153);font-weight:bold">void</span> foo() { printf(<span style="background-color:rgb(255,240,240)">"%s</span><span style="color:rgb(102,102,102);font-weight:bold;background-color:rgb(255,240,240)">\n</span><span style="background-color:rgb(255,240,240)">"</span>, __PRETTY_FUNCTION__); }
<span style="color:rgb(51,51,153);font-weight:bold">int</span> m;
<span style="color:rgb(51,51,153);font-weight:bold">int</span> *<span style="color:rgb(0,102,187);font-weight:bold">zip</span>() { <span style="color:rgb(0,136,0);font-weight:bold">return</span> &m; }
};
<span style="color:rgb(0,136,0);font-weight:bold">struct</span> B : A {
<span style="color:rgb(0,136,0);font-weight:bold">virtual</span> <span style="color:rgb(51,51,153);font-weight:bold">void</span> foo() { printf(<span style="background-color:rgb(255,240,240)">"%s</span><span style="color:rgb(102,102,102);font-weight:bold;background-color:rgb(255,240,240)">\n</span><span style="background-color:rgb(255,240,240)">"</span>, __PRETTY_FUNCTION__); }
};
<span style="color:rgb(51,51,153);font-weight:bold">int</span> <span style="color:rgb(0,102,187);font-weight:bold">main</span>(<span style="color:rgb(51,51,153);font-weight:bold">void</span>) {
A *ap = <span style="color:rgb(0,136,0);font-weight:bold">new</span> A;
ap->foo();
<span style="color:rgb(51,51,153);font-weight:bold">int</span> *<span style="color:rgb(0,136,0);font-weight:bold">const</span> apz = ap->zip();
B *bp = <span style="color:rgb(0,136,0);font-weight:bold">new</span> (ap) B;
<span style="color:rgb(0,136,0);font-weight:bold">if</span> (apz == bp->zip()) {
bp->foo();
}
}</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><br></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">Here we don't compare vptrs, but pointers with small offset like:</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">a+4 == b+4</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif"><br></font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">And because we can transform it to a == b, it breaks.</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif"><br></font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">I started thinking about handling it in the LLVM instead, and my first idea was as folows:</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">Because the invariant.group is associated with the pointer value, preserving the invariant.group information when changing pointer seems invalid.</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">If we replace one pointer with another, based on the comparision, we could strip all of the invariant.group information from it.</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">This idea is good enough to solve the above code, but it not gonna work for all the cases, like passing the pointer to a function or returning it - </font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">by doing later inlining we could get the same situation and we would not know that we should strip invariant.group metadata.</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif"><br></font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">Other idea is following:</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">We can replace the pointer by another pointer after barrier like:</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="monospace, monospace"><br></font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="monospace, monospace">if (a == b)</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="monospace, monospace"> use(b)</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="monospace, monospace">=></font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="monospace, monospace">if (a == b)</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="monospace, monospace"> use(barrier(a))</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif"><br></font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">This would be probablly correct, but I am not sure if it is a good solution, because It might actually make it worse from optimization point of view.</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">I am also not sure what should be the semantics of the barrier(constant) - should we constantfold it to constant? I am not sure if constant pointer </font></pre><pre style="margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif"><font style="color:rgb(51,51,51)">can carry any invariant.group information - I am pretty sure that <b>null</b> and <b>undef</b> does not carry it (</font><font color="#333333"><a href="https://reviews.llvm.org/D32423">https://reviews.llvm.org/D32423</a></font><font color="#333333">)</font><font color="#333333">.</font></font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif"><br></font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">The third solution I am thinking of is just not to propagate the new value to every dominated use - </font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">especially to instructions like calls, return or load/store with invariant.group information. </font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">To validate this idea I need some help from GVN wizzards - would it break any important optimizations, etc?</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">Please help.</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif"><br></font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif"><br></font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><br></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">Also, Sanjoy proposed to mark invariant.group features as experimental, so we will not be afraid to break behavior of frontends that already use it. </font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">Right now I am pretty sure that clang is the only one that curently uses it (and not by default).</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">Is anyone ok with it?</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif"><br></font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><br></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">Best</font></pre><pre style="color:rgb(51,51,51);margin-top:0px;margin-bottom:0px;line-height:16.25px"><font face="arial, helvetica, sans-serif">Piotr</font></pre></div></div>