[llvm-dev] Hoisting in the presence of volatile loads.

[Alina Sbirlea via llvm-dev]

On Dec 22, 2017 15:53, "Philip Reames" <listmail at philipreames.com> wrote:



On 12/20/2017 09:17 PM, Hal Finkel wrote:


On 12/20/2017 03:49 PM, Alina Sbirlea via llvm-dev wrote:

+Philip to get his input too.
I've talked with George offline, and here's a summary:

In D16875 <https://reviews.llvm.org/D16875>, the decision made was: "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)"
So the testcase: test/Analysis/MemorySSA/volatile-clobber.ll, is checking
that a volatile load is the defining access of a load with which it may
alias.

Snippet:
; Ensuring that we don't automatically hoist nonvolatile loads around
volatile
; loads
; CHECK-LABEL define void @volatile_only
define void @volatile_only(i32* %arg1, i32* %arg2) {
[...]
; MayAlias
; CHECK: 2 = MemoryDef(1)
; CHECK-NEXT: load volatile i32, i32* %arg1
  load volatile i32, i32* %arg1
; CHECK: MemoryUse(2)
; CHECK-NEXT: load i32, i32* %arg2
  load i32, i32* %arg2
  ret void
}

The testcase: test/Transforms/LICM/volatile-alias.ll checks the opposite,
that we *do* hoist the non-volatile load. This is currently ensured by the
AliasSetTracker.


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.
So, how can I make progress here?


I think it's reasonable in D40375 <https://reviews.llvm.org/D40375> to have
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.

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.
George's 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?


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:

The optimizers must not change the number of volatile operations or change
their order of execution relative to other volatile operations. The
optimizers *may* change the order of volatile operations relative to
non-volatile operations. This is not Java’s “volatile” and has no
cross-thread synchronization behavior.


I see no reason to prevent this kind of reordering, even if the volatile
and non-volatile accesses might alias.

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.


Thank you for the reply and confirming the approach!

Best,
Alina



 -Hal


Thoughts?


Thanks,
Alina


On Wed, Dec 20, 2017 at 11:50 AM, Krzysztof Parzyszek <
kparzysz at codeaurora.org> wrote:

> On 12/20/2017 1:37 PM, Sanjoy Das wrote:>
>
>> Fwiw, I was under the impression that regular loads could *not* be
>> reordered with volatile loads since we could have e.g.:
>>
>>    int *normal = &global_variable;
>>    volatile int* ptr = 0;
>>    int k = *ptr; // segfaults, and the signal handler writes to *normal
>>    int value = *normal;
>>
>> and that we'd have to treat volatile loads and stores essentially as
>> calls to unknown functions.
>>
>
> For this to work, "normal" should be volatile as well.
>
> -Krzysztof
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by The Linux Foundation
>



_______________________________________________
LLVM Developers mailing
listllvm-dev at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev


-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171222/5e136346/attachment.html>