<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Apr 8, 2014 at 9:14 PM, Karthik Bhat <span dir="ltr"><<a href="mailto:blitz.opensource@gmail.com" target="_blank">blitz.opensource@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Thanks Dan for the review. I have a few doubts though if you could clarify the same.<br>
<div class="gmail_extra"><br><br><div class="gmail_quote"><div class="">On Wed, Apr 9, 2014 at 12:17 AM, Dan Gohman <span dir="ltr"><<a href="mailto:dan433584@gmail.com" target="_blank">dan433584@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">
<div>On Tue, Apr 8, 2014 at 5:04 AM, Karthik Bhat <span dir="ltr"><<a href="mailto:kv.bhat@samsung.com" target="_blank">kv.bhat@samsung.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi majnemer, sunfish, rengolin,<br>
<br>
Hi All,<br>
This patches fixes PR16236 aproblem with TypeBasedAlias Analysis. Below is my understanding please help me review this patch and if i'm thinking in the right direction.<br>
The problem in this case seems to be that in case we have an union like -<br>
union<br>
{<br>
short f0;<br>
int f1;<br>
} u, *up = &u;<br>
<br>
f0 and f1 may alias each other. But while constructing AliasSet for these in TypeBasedAlias Analysis during PathAliases we see that both the MDNodes corresponding to (f0 and f1) are not ancestor of each other and have a common root and thus conclude that they will Not alias each other. But in case of union this may not be correct f1 anf f0 may alias each other as thye operate on same underlining memory location.<br>
</blockquote><div><br></div></div><div>LLVM's approach for solving this problem is to do a BasicAA query first, and if BasicAA says MustAlias or PartialAlias, it doesn't do a TBAA query at all. This represents a kind of "best effort": the optimizer will make its best effort to discover whether the type information is unreliable, but if it doesn't discover a problem, then it assumes that the type information is reliable. As such, it's not safe in all cases, but the C standard's own definition of TBAA is not safe in all cases, and this is an attempt to implement the spirit of what the C standard desired while finding a practical balance with correctness.<br>
</div><div><br>Are you seeing a case where this approach is not sufficient?<br></div></div></div></div></blockquote><div><br></div></div><div style="font-family:arial,sans-serif;font-size:13px">Yes Dan in PR16236. <a href="http://llvm.org/bugs/show_bug.cgi?id=16236" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=16236</a> we have an example were the behavior with and without optimization is different. Ideally optimization should not change program behavior. Given the below program from the bugID-</div>
<div style="font-family:arial,sans-serif;font-size:13px"><pre style="white-space:pre-wrap;width:50em">int printf(const char *, ...);
union
{
short f0;
int f1;
} u, *up = &u;
int a;
int main ()
{
for (; a <= 0; a++)
if ((u.f0 = 1))
up->f1 = 0;
printf ("%d\n", u.f1);
return 0;
}</pre></div><div><span style="font-family:arial,sans-serif;font-size:13px">TypeBasedAlias analysis incorrectly returns NoAlias for u.f0 and up->f1</span></div></div></div></div></blockquote><div><br></div><div><br></div>
<div>Actually, at least by the C99 rules, this is not incorrect.</div><div><br></div><div>Annex J.1 says "The following are unspecified:</div><div>...</div><div><div>— The value of a union member other than the last one stored into (6.2.6.1).</div>
</div><div>"</div><div>6.2.6.1 says:</div><div>"When a value is stored in a member of an object of union type, the bytes of the object</div><div>representation that do not correspond to that member but do correspond to other members</div>
<div>take unspecified values."</div><div><br></div><div>When you store f0, the value of f1 also becomes unspecified.</div><div><br></div><div>Now, as Dan says, compilers essentially do the best they can to make this "not crazy".</div>
<div><br></div><div>But you can come up with arbitrarily complex examples where most TBAA is going to fall down and claim noalias, even though BasicAA would say MustAlias. A trivial variation on your example :<br></div><div>
<br></div><div><pre style="font-size:13px;white-space:pre-wrap;width:50em">union
{
short f0;
int f1;
} u, *up = &u;
</pre></div><div><br></div><div>int foo(short *a, int *b) {</div><div>// Within this function, *a and *b will be considered disjoint by most TBAA implementations that implement "C rules", or things that produce TBAA sets that implement them.</div>
<div>// This is not a great example, for various reasons, but you can see where it is going.</div><div><br></div><div>}</div><div><br></div><div>int main(void)</div><div>{</div><div> foo(&u.f0, &u.f1);<br></div>
<div>}<br><br></div><div><br></div><div>Now place foo in a different translation unit.</div><div>Now there is no way compiler could know they alias.<br></div><div><br></div><div>GCC is a little more conservative here, and says "if we see you place the two types in a union, we assume pointers to them could alias". </div>
<div><br></div><div>But if you place it in a different TU, it also gives up.</div><div><br></div><div>As Dan says, the C standard's "definition" of aliasing is knowingly deficient in a large number of ways that it make it impossible for compilers to use safely in all cases.</div>
<div>They all do the best they can.</div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div></div></div></div>