<div dir="auto"><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Dec 22, 2017 15:53, "Philip Reames" <<a href="mailto:listmail@philipreames.com">listmail@philipreames.com</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"><div class="elided-text">
<p><br>
</p>
<br>
<div class="m_-8742523396977938305moz-cite-prefix">On 12/20/2017 09:17 PM, Hal Finkel
wrote:<br>
</div>
<blockquote type="cite">
<p><br>
</p>
<div class="m_-8742523396977938305moz-cite-prefix">On 12/20/2017 03:49 PM, Alina Sbirlea
via llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">+Philip to get his input too.
<div>I've talked with George offline, and here's a summary:<br>
<div><br>
</div>
<div>In <a href="https://reviews.llvm.org/D16875" target="_blank">D16875</a>, the
decision made was: "<span>The
LLVM spec is ambiguous about whether we can hoist a
non-volatile load above a volatile load when the loads
alias. It's probably best not to exploit this ambiguity
at the moment by unconditionally allowing the motion of
nonvolatile loads above volatile loads (and vice versa)"</span></div>
</div>
<div><span>So
the testcase: </span><span style="font-size:12.8px">test/Analysis/<wbr>MemorySSA/</span><span style="font-size:12.8px">volatile-clobber.ll,
is checking that a volatile load is the defining access of
a load with which it may alias.</span></div>
<div><span style="font-size:12.8px"><br>
</span></div>
<div><span style="font-size:12.8px">Snippet:</span></div>
<div>
<div><span style="font-size:12.8px">; Ensuring that we don't
automatically hoist nonvolatile loads around volatile</span></div>
<div><span style="font-size:12.8px">; loads</span></div>
<div><span style="font-size:12.8px">; CHECK-LABEL define
void @volatile_only</span></div>
<div><span style="font-size:12.8px">define void
@volatile_only(i32* %arg1, i32* %arg2) {</span></div>
</div>
<div><span style="font-size:12.8px">[...]</span></div>
<div>
<div><span style="font-size:12.8px">; MayAlias</span></div>
<div><span style="font-size:12.8px">; CHECK: 2 =
MemoryDef(1)</span></div>
<div><span style="font-size:12.8px">; CHECK-NEXT: load
volatile i32, i32* %arg1</span></div>
<div><span style="font-size:12.8px"> load volatile i32,
i32* %arg1</span></div>
<div><span style="font-size:12.8px">; CHECK: MemoryUse(2)</span></div>
<div><span style="font-size:12.8px">; CHECK-NEXT: load i32,
i32* %arg2</span></div>
<div><span style="font-size:12.8px"> load i32, i32* %arg2</span></div>
</div>
<div>
<div><span style="font-size:12.8px"> ret void</span></div>
<div><span style="font-size:12.8px">}</span></div>
</div>
<div><span style="font-size:12.8px"><br>
</span></div>
<div><span style="font-size:12.8px">The testcase</span>:
test/Transforms/LICM/volatile-<wbr>alias.ll <span style="font-size:12.8px">checks the opposite, that we *do*
hoist the non-volatile load. This is currently ensured by
the AliasSetTracker.</span></div>
<div><span style="font-size:12.8px"><br>
</span></div>
<div><span style="font-size:12.8px"><br>
</span></div>
<div><span style="font-size:12.8px">The conclusion I'm drawing
is that right now AliasSetTracker and MemorySSA have
different behaviors and replacing one with the either will
naturally lead to different outcomes. </span></div>
<div><span style="font-size:12.8px">So, how can I make
progress here?</span></div>
<div><br>
</div>
<div><span style="font-size:12.8px"><br>
</span></div>
<div>I think it's reasonable in <a href="https://reviews.llvm.org/D40375" style="font-size:12.8px" target="_blank">D40375</a><span style="font-size:12.8px"> to have</span> <wbr>pointerInvalidatedByLoopWithMS<wbr>SA
only check if the defining access is within the current loop
or liveOnEntry, and rely on MemorySSA to either consider a
volatile load a clobbering access or not. So, right now the
LICM/volatile-alias.ll <wbr>testcase will behave differently with
MemorySSA enabled.</div>
<div><br>
</div>
A separate decision, is whether to update
getLoadReorderability in MemorySSA to remove the restriction
that loads should not alias. That would give the same behavior
as the AliasSetTracker.
<div><span>George's </span><span style="font-size:12.8px">recollection is that the reason
MemorySSA didn't reorder aggressively is because it was
untested at the time. Now that MemorySSA is used more
widely, it may be a good time to revisit the initial
decision?</span></div>
</div>
</blockquote>
<br>
I think that this is the right way to approach this: we should
change MemorySSA to be less conservative in this regard. LLVM's
language reference is pretty explicit about reordering volatile
and non-volatile operations:<br>
<br>
<blockquote type="cite">
<span>The optimizers must not change the number of
volatile operations or change their order of execution
relative to other volatile operations. The optimizers<span> </span></span><em>may</em><span><span> </span>change the order of volatile
operations relative to non-volatile operations. This is not
Java’s “volatile” and has no cross-thread synchronization
behavior.</span></blockquote>
<br>
I see no reason to prevent this kind of reordering, even if the
volatile and non-volatile accesses might alias.<br>
</blockquote></div>
Just to chime in here since I was explicitly CCd, I agree with this
conclusion and believe this thread reached the proper result per the
appropriate specs.</div></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">Thank you for the reply and confirming the approach!</div><div dir="auto"><br></div><div dir="auto">Best,</div><div dir="auto">Alina</div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF"><div class="elided-text"><br>
<blockquote type="cite"> <br>
-Hal<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div><span style="font-size:12.8px"><br>
</span></div>
<div><span style="font-size:12.8px">Thoughts?</span></div>
<div><span style="font-size:12.8px"><br>
</span></div>
<div><span style="font-size:12.8px"><br>
</span></div>
<div><span style="font-size:12.8px">Thanks,</span></div>
<div><span style="font-size:12.8px">Alina<br>
</span>
<div>
<div><span><br>
</span></div>
</div>
</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Dec 20, 2017 at 11:50 AM,
Krzysztof Parzyszek <span dir="ltr"><<a href="mailto:kparzysz@codeaurora.org" target="_blank">kparzysz@codeaurora.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On 12/20/2017 1:37 PM, Sanjoy Das wrote:><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Fwiw, I was under the impression that regular loads
could *not* be<br>
reordered with volatile loads since we could have
e.g.:<br>
<br>
int *normal = &global_variable;<br>
volatile int* ptr = 0;<br>
int k = *ptr; // segfaults, and the signal handler
writes to *normal<br>
int value = *normal;<br>
<br>
and that we'd have to treat volatile loads and stores
essentially as<br>
calls to unknown functions.<br>
</blockquote>
<br>
</span> For this to work, "normal" should be volatile as
well.<span class="m_-8742523396977938305HOEnZb"><font color="#888888"><br>
<br>
-Krzysztof</font></span>
<div class="m_-8742523396977938305HOEnZb">
<div class="m_-8742523396977938305h5"><br>
<br>
-- <br>
Qualcomm Innovation Center, Inc. is a member of Code
Aurora Forum, hosted by The Linux Foundation<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
<br>
<fieldset class="m_-8742523396977938305mimeAttachmentHeader"></fieldset>
<br>
<pre>______________________________<wbr>_________________
LLVM Developers mailing list
<a class="m_-8742523396977938305moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>
<a class="m_-8742523396977938305moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
<br>
<pre class="m_-8742523396977938305moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</blockquote>
<br>
</div></div>
</blockquote></div><br></div></div></div>