<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Aug 9, 2017 at 8:37 PM, 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_-1753555538924036895moz-cite-prefix">On 08/09/2017 10:14 PM, Xinliang David
Li via llvm-commits wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Aug 9, 2017 at 7:42 PM,
Chandler Carruth via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">chandlerc
added a comment.<br>
<br>
This has been discussed before and I still pretty strongly
disagree with it.<br>
<br>
This cripples the ability of TSan to find race conditions
between accesses to consecutive bitfields -- and these
bugs have actually come up.<br>
<br>
</blockquote>
<div><br>
</div>
<div><br>
</div>
<div>Can you elaborate on this? Do you mean race conditions
due to widening?</div>
<div><br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
We also have had cases in the past where LLVM missed
significant bitfield combines and optimizations due to
loading them as separate integers. Those would become
problems again, and I think they would be even harder to
solve than narrowing the access is going to be because we
will have strictly less information to work with.<br>
<br>
</blockquote>
<div><br>
</div>
<div> Can you elaborate here too? If there were missed
optimization that later got fixed, there should be
regression tests for them, right? And what information is
missing?</div>
</div>
</div>
</div>
</blockquote>
<br></span>
To make a general statement, if we load (a, i8) and (a+2, i16), for
example, and these came from some structure, we've lost the
information that the load (a+1, i8) would have been legal (i.e. is
known to be deferenceable). This is not specific to bit fields, but
the fact that we lose information on the dereferenceable byte ranges
around memory access turns into a problem when we later can't
legally widen. There may be a better way to keep this information
other than producing wide loads (which is an imperfect mechanism,
especially the way we do it by restricting to legal integer types),
but at the moment, we don't have anything better.<br></div></blockquote><div><br></div><div>Ok, as you mentioned, widening looks like a workaround to paper over the weakness in IR to annotate the information. More importantly, my question is whether this is a just theoretical concern.</div><div><br></div><div>If this is an important issue to avoid before we have a good IR, it seems that we can always safely do this if there is no 'gap' between the bitfield candidates.</div><div><br></div><div>David</div><div> </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>
-Hal<br>
<br>
<blockquote type="cite"><span class="">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
</div>
<div>thanks,</div>
<div><br>
</div>
<div>David</div>
<div><br>
</div>
<div><br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Ultimately, while I understand the appeal of this
approach, I don't think it is correct and I think we
should instead figure out how to optimize these memory
accesses well in LLVM. That approach will have the added
benefit of optimizing cases where the user has manually
used a large integer to simulate bitfields, and making
combining and canonicalization substantially easier.<br>
<div class="m_-1753555538924036895HOEnZb">
<div class="m_-1753555538924036895h5"><br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D36562" rel="noreferrer" target="_blank">https://reviews.llvm.org/D3656<wbr>2</a><br>
<br>
<br>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
<br>
<fieldset class="m_-1753555538924036895mimeAttachmentHeader"></fieldset>
<br>
</span><pre>______________________________<wbr>_________________
llvm-commits mailing list
<a class="m_-1753555538924036895moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>
<a class="m_-1753555538924036895moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><span class="HOEnZb"><font color="#888888">
</font></span></pre><span class="HOEnZb"><font color="#888888">
</font></span></blockquote><span class="HOEnZb"><font color="#888888">
<br>
<pre class="m_-1753555538924036895moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</font></span></div>
</blockquote></div><br></div></div>