<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 12/20/2017 09:17 PM, Hal Finkel
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:a3a4d990-5751-faa0-66ac-9ca0ff20d378@anl.gov">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<p><br>
</p>
<div class="moz-cite-prefix">On 12/20/2017 03:49 PM, Alina Sbirlea
via llvm-dev wrote:<br>
</div>
<blockquote
cite="mid:CAAtaGTqiMCBU0O10BfNrRYZGpfyOS0tmig+YJhU4Gz_-eVtbnA@mail.gmail.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8">
<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 moz-do-not-send="true"
href="https://reviews.llvm.org/D16875">D16875</a>, the
decision made was: "<span
style="color:rgb(0,0,0);font-family:"Segoe
UI","Segoe UI Emoji","Segoe UI
Symbol",Lato,"Helvetica
Neue",Helvetica,Arial,sans-serif;font-size:13px">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 style="color:rgb(0,0,0);font-family:"Segoe
UI","Segoe UI Emoji","Segoe UI
Symbol",Lato,"Helvetica
Neue",Helvetica,Arial,sans-serif;font-size:13px">So
the testcase: </span><span style="font-size:12.8px">test/Analysis/MemorySSA/</span><wbr
style="font-size:12.8px"><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-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 moz-do-not-send="true"
href="https://reviews.llvm.org/D40375"
style="font-size:12.8px">D40375</a><span
style="font-size:12.8px"> to have</span> pointerInvalidatedByLoopWithMSSA
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 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
style="font-size:13px;color:rgb(0,0,0);font-family:"Segoe
UI","Segoe UI Emoji","Segoe UI
Symbol",Lato,"Helvetica
Neue",Helvetica,Arial,sans-serif">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">
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
<span style="color: rgb(0, 0, 0); font-family: "Lucida
Grande", "Lucida Sans Unicode", Geneva,
Verdana, sans-serif; font-size: 14px; font-style: normal;
font-variant-ligatures: normal; font-variant-caps: normal;
font-weight: 400; letter-spacing: normal; orphans: 2;
text-align: left; text-indent: 0px; text-transform: none;
white-space: normal; widows: 2; word-spacing: 0px;
-webkit-text-stroke-width: 0px; background-color: rgb(255,
255, 255); text-decoration-style: initial;
text-decoration-color: initial; display: inline !important;
float: none;">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; orphans: 2; text-align: left;
text-indent: 0px; text-transform: none; white-space: normal;
widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial;">may</em><span
style="color: rgb(0, 0, 0); font-family: "Lucida
Grande", "Lucida Sans Unicode", Geneva,
Verdana, sans-serif; font-size: 14px; font-style: normal;
font-variant-ligatures: normal; font-variant-caps: normal;
font-weight: 400; letter-spacing: normal; orphans: 2;
text-align: left; text-indent: 0px; text-transform: none;
white-space: normal; widows: 2; word-spacing: 0px;
-webkit-text-stroke-width: 0px; background-color: rgb(255,
255, 255); text-decoration-style: initial;
text-decoration-color: initial; display: inline !important;
float: none;"><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>
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.<br>
<blockquote type="cite"
cite="mid:a3a4d990-5751-faa0-66ac-9ca0ff20d378@anl.gov"> <br>
-Hal<br>
<br>
<blockquote
cite="mid:CAAtaGTqiMCBU0O10BfNrRYZGpfyOS0tmig+YJhU4Gz_-eVtbnA@mail.gmail.com"
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 style="color:rgb(0,0,0);font-family:"Segoe
UI","Segoe UI Emoji","Segoe UI
Symbol",Lato,"Helvetica
Neue",Helvetica,Arial,sans-serif;font-size:13px"><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
moz-do-not-send="true"
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
class="">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="HOEnZb"><font color="#888888"><br>
<br>
-Krzysztof</font></span>
<div class="HOEnZb">
<div class="h5"><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="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
LLVM Developers mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org" moz-do-not-send="true">llvm-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</blockquote>
<br>
</body>
</html>