<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sun, May 14, 2017 at 11:01 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"><div><div class="h5">
<p><br>
</p>
<div class="m_-6131269866192422863moz-cite-prefix">On 05/14/2017 12:49 PM, Daniel Berlin
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Sun, May 14, 2017 at 10:20 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">
<div>
<div class="m_-6131269866192422863h5">
<p><br>
</p>
<div class="m_-6131269866192422863m_-7109946281551088390moz-cite-prefix">On
05/14/2017 11:06 AM, Daniel Berlin wrote:<br>
</div>
<blockquote type="cite">
<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>
<p><br>
</p>
<div class="m_-6131269866192422863m_-7109946281551088390m_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_bug<wbr>.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>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</div>
I don't see why need to break backward compatibility. Do
you have something specific in mind? Once we extend the
TBAA implementation to treat repeated offsets as
disjunction, then we'll extend Clang to emit metadata
for unions in this form. Old IR will work exactly as it
does now.</div>
</blockquote>
<div> </div>
<div>Except the Old IR is irretrievably broken. and in this
method, you can't tell whether you are dealing with
correct TBAA or not.</div>
<div><br>
</div>
<div>Earlier, the discussion was basically "detect old IR,
disable TBAA since it's broken, make new IR work".</div>
<div><br>
</div>
<div>If we do that, we need a way to distinguish new vs old
IR.</div>
</div>
</div>
</div>
</blockquote>
<br>
<br></div></div>
Ah, okay. I don't think that's desirable in general. There are
frontends emitting perfectly self-consistent TBAA metadata, and
there's no reason they should change. Clang's metadata is broken
because the mapping between language rules and TBAA is broken. If
we'd like, we can blacklist TBAA metadata coming from root nodes
named "Simple C++ TBAA" and "Simple C/C++ TBAA" (or provide an
option to do that because I'm not convinced it is worth trying to
retroactively "fix" old IR by default given the associated
performance penalties). After the fix, Clang can emit root nodes
with different names (they're arbitrary). Changing the root-node
names will also give the conservatively-correct behavior when
linking old IR with new IR.<span class=""><br>
<br></span></div></blockquote><div><br></div><div>I still continue to believe trying to patch the existing mechanism this way is not a great plan, and likely to end us in a place where we still have either correctness or performance issues.</div><div>But i'm not doing the work, so if that's what you guys want to do, go for it.<br></div><div><br></div><div>--Dan</div></div></div></div>