<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sun, May 14, 2017 at 8:37 AM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span class="">
<p><br>
</p>
<div class="m_7871019985833609421moz-cite-prefix">On 03/01/2017 05:30 PM, Daniel Berlin
via llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">So, <a href="https://bugs.llvm.org/show_bug.cgi?id=32056" target="_blank">https://bugs.llvm.org/show_<wbr>bug.cgi?id=32056</a>
is an example showing our current TBAA tree for union generation
is definitely irretrievably broken.
<div>I'll be honest here. I'm pretty sure your proposal doesn't
go far enough.</div>
<div>But truthfully, I would rather see us come closer to a
representation we know works, which is GCC's.</div>
<div>Let me try to simplify what you are suggesting, and what we
have.</div>
<div>Our current representation is basically inverted from GCC,
but we don't allow things that would enable it to work.<br>
</div>
<div><br>
</div>
<div>Given</div>
<div>union {int a, short b};</div>
<div><br>
</div>
<div>GCC's will be:</div>
<div><br>
</div>
<div> union</div>
<div> / \</div>
<div>short int</div>
<div><br>
</div>
<div><br>
</div>
<div>Everything is implicitly a subset of alias set 0 (which for
C/C++ is char) just to avoid representing it.</div>
<div><br>
</div>
<div>Our metadata has no child links, and only a single parent
link.</div>
<div><br>
</div>
<div>You can't represent the above form because you can't make a
single short node a child of every union/struct it needs to
be (lack of multiple parents means you can't just frob them
all to offset zero).</div>
<div><br>
</div>
<div>Your suggestion is to invert this as a struct</div>
<div><br>
</div>
<div>short int</div>
<div> | / </div>
<div>union</div>
<div><br>
</div>
<div>We don't allow multiple parents, however.</div>
<div>Because of that, you suggest we follow all nodes for
unions, special casing union-type nodes somehow</div>
</div>
</blockquote>
<br></span>
Now that I've spent a bunch of time looking at this, I'd like to
voice support for Steven's original proposal. In the context of what
we have, it makes sense, seems sound, and should fix the
representational mapping problem we currently have. </div></blockquote><div><br></div><div>Except you can't easily differentiate it from the current one, and if we are going to have to upgrade/break compatibility, why not just do it once right, a way we know works, instead of risk screwing it up again, and playing with a representation we aren't actually sure we can make efficient for this case?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">Something
completely different (e.g. closer to what GCC uses) can work too,
but this seems unnecessary (the proposed extension to the current
semantics seem equivalently expressive).<br></div></blockquote><div><br></div><div>Yes, they are equivalently expressive.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">
<br>
What you call "special casing of union-type nodes" does not actually
seem all that special. The rule seems quite simple: if you some
across duplicate offsets, then search all of them. </div></blockquote><div><br></div><div>Which means you have to know they exist, for starters, which means keeping it in some sorted order and checking, or some other mechanism.</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">This should not
be difficult to implement or use, and seems no less efficient than
any other way of encoding the concept of disjunction in the
hierarchy.<span class="HOEnZb"><font color="#888888"><br>
<br></font></span></div></blockquote><div>It is, in actuality, less efficient. </div><div>For starters, in the inverted representation, you don't have to explicitly represent the things that are children of alias set zero (char in C++'s case), which is quite common.</div><div><br></div><div>It's also trivial to generate transitive closures of parts of the graph in the inverted representation, and be able to use them, if you need to.</div><div>It turns out to be much trickier the other way.</div><div><br></div><div>Those are just trivial examples.</div><div><br></div><div>Now, how often this matters, don't know.</div><div><br></div><div>But i'm suggesting what i believe to be the most practical route:</div><div>Given a situation where our representation has been broken for years, take an approach that is battle tested and we know works and is efficient, instead of trying to patch our representation and hope we've thought it through well enough :)</div><div><br></div><div>Does this mean the original proposal won't work?</div><div>Nope. It may in fact work. But i'd still do the thing i knew already worked well. Because like i said, you are going to break compatibility anyway, so ...</div><div><br></div></div></div></div>