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

[Alina Sbirlea via llvm-dev]

+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?

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20171220/898bf4c1/attachment.html>