<div dir="ltr">Thank you for the lang ref pointer, Hal!<div>I sent out the change in <a href="https://reviews.llvm.org/D41525">D41525</a>.</div><div><br></div><div>Best,</div><div>Alina</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Dec 20, 2017 at 9:17 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"><div><div class="h5">
<p><br>
</p>
<div class="m_-428978424249800317moz-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></div></div>
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 style="color:rgb(0,0,0);font-family:"Lucida Grande","Lucida Sans Unicode",Geneva,Verdana,sans-serif;font-size:14px;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:left;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial">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>
<br>
-Hal<br>
<br>
<blockquote type="cite"><span class="">
<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_-428978424249800317HOEnZb"><font color="#888888"><br>
<br>
-Krzysztof</font></span>
<div class="m_-428978424249800317HOEnZb">
<div class="m_-428978424249800317h5"><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_-428978424249800317mimeAttachmentHeader"></fieldset>
<br>
</span><pre>______________________________<wbr>_________________
LLVM Developers mailing list
<a class="m_-428978424249800317moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>
<a class="m_-428978424249800317moz-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><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_-428978424249800317moz-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>