diff options
author | Christof Douma <Christof.Douma@arm.com> | 2019-03-18 09:21:06 +0000 |
---|---|---|
committer | Christof Douma <Christof.Douma@arm.com> | 2019-03-18 09:21:06 +0000 |
commit | 8cfd91dcc726307c2d35d17f5e13b9ac047c2187 (patch) | |
tree | 2279703073adc2d807469bdba42d1155410dd8b4 /llvm/lib | |
parent | ba898da132d4b4f6eeb5211e94cd141458383eee (diff) | |
download | bcm5719-llvm-8cfd91dcc726307c2d35d17f5e13b9ac047c2187.tar.gz bcm5719-llvm-8cfd91dcc726307c2d35d17f5e13b9ac047c2187.zip |
[AArch64] Fix bug 35094 atomicrmw on Armv8.1-A+lse
Fixes https://bugs.llvm.org/show_bug.cgi?id=35094
The Dead register definition pass should leave alone the atomicrmw
instructions on AArch64 (LTE extension). The reason is the following
statement in the Arm ARM:
"The ST<OP> instructions, and LD<OP> instructions where the destination
register is WZR or XZR, are not regarded as doing a read for the purpose
of a DMB LD barrier."
A good example was given in the gcc thread by Will Deacon (linked in the
bugzilla ticket 35094):
P0 (atomic_int* y,atomic_int* x) {
atomic_store_explicit(x,1,memory_order_relaxed);
atomic_thread_fence(memory_order_release);
atomic_store_explicit(y,1,memory_order_relaxed);
}
P1 (atomic_int* y,atomic_int* x) {
atomic_fetch_add_explicit(y,1,memory_order_relaxed); // STADD
atomic_thread_fence(memory_order_acquire);
int r0 = atomic_load_explicit(x,memory_order_relaxed);
}
P2 (atomic_int* y) {
int r1 = atomic_load_explicit(y,memory_order_relaxed);
}
My understanding is that it is forbidden for r0 == 0 and r1 == 2 after
this test has executed. However, if the relaxed add in P1 compiles to
STADD and the subsequent acquire fence is compiled as DMB LD, then we
don't have any ordering guarantees in P1 and the forbidden result could
be observed.
Change-Id: I419f9f9df947716932038e1100c18d10a96408d0
llvm-svn: 356360
Diffstat (limited to 'llvm/lib')
-rw-r--r-- | llvm/lib/Target/AArch64/AArch64DeadRegisterDefinitionsPass.cpp | 47 |
1 files changed, 46 insertions, 1 deletions
diff --git a/llvm/lib/Target/AArch64/AArch64DeadRegisterDefinitionsPass.cpp b/llvm/lib/Target/AArch64/AArch64DeadRegisterDefinitionsPass.cpp index c0888badb23..a43077cb88e 100644 --- a/llvm/lib/Target/AArch64/AArch64DeadRegisterDefinitionsPass.cpp +++ b/llvm/lib/Target/AArch64/AArch64DeadRegisterDefinitionsPass.cpp @@ -68,6 +68,51 @@ static bool usesFrameIndex(const MachineInstr &MI) { return false; } +// Instructions that lose their 'read' operation for a subesquent fence acquire +// (DMB LD) once the zero register is used. +// +// WARNING: The aquire variants of the instructions are also affected, but they +// are split out into `atomicBarrierDroppedOnZero()` to support annotations on +// assembly. +static bool atomicReadDroppedOnZero(unsigned Opcode) { + switch (Opcode) { + case AArch64::LDADDB: case AArch64::LDADDH: + case AArch64::LDADDW: case AArch64::LDADDX: + case AArch64::LDADDLB: case AArch64::LDADDLH: + case AArch64::LDADDLW: case AArch64::LDADDLX: + case AArch64::LDCLRB: case AArch64::LDCLRH: + case AArch64::LDCLRW: case AArch64::LDCLRX: + case AArch64::LDCLRLB: case AArch64::LDCLRLH: + case AArch64::LDCLRLW: case AArch64::LDCLRLX: + case AArch64::LDEORB: case AArch64::LDEORH: + case AArch64::LDEORW: case AArch64::LDEORX: + case AArch64::LDEORLB: case AArch64::LDEORLH: + case AArch64::LDEORLW: case AArch64::LDEORLX: + case AArch64::LDSETB: case AArch64::LDSETH: + case AArch64::LDSETW: case AArch64::LDSETX: + case AArch64::LDSETLB: case AArch64::LDSETLH: + case AArch64::LDSETLW: case AArch64::LDSETLX: + case AArch64::LDSMAXB: case AArch64::LDSMAXH: + case AArch64::LDSMAXW: case AArch64::LDSMAXX: + case AArch64::LDSMAXLB: case AArch64::LDSMAXLH: + case AArch64::LDSMAXLW: case AArch64::LDSMAXLX: + case AArch64::LDSMINB: case AArch64::LDSMINH: + case AArch64::LDSMINW: case AArch64::LDSMINX: + case AArch64::LDSMINLB: case AArch64::LDSMINLH: + case AArch64::LDSMINLW: case AArch64::LDSMINLX: + case AArch64::LDUMAXB: case AArch64::LDUMAXH: + case AArch64::LDUMAXW: case AArch64::LDUMAXX: + case AArch64::LDUMAXLB: case AArch64::LDUMAXLH: + case AArch64::LDUMAXLW: case AArch64::LDUMAXLX: + case AArch64::LDUMINB: case AArch64::LDUMINH: + case AArch64::LDUMINW: case AArch64::LDUMINX: + case AArch64::LDUMINLB: case AArch64::LDUMINLH: + case AArch64::LDUMINLW: case AArch64::LDUMINLX: + return true; + } + return false; +} + void AArch64DeadRegisterDefinitions::processMachineBasicBlock( MachineBasicBlock &MBB) { const MachineFunction &MF = *MBB.getParent(); @@ -88,7 +133,7 @@ void AArch64DeadRegisterDefinitions::processMachineBasicBlock( continue; } - if (atomicBarrierDroppedOnZero(MI.getOpcode())) { + if (atomicBarrierDroppedOnZero(MI.getOpcode()) || atomicReadDroppedOnZero(MI.getOpcode())) { LLVM_DEBUG(dbgs() << " Ignoring, semantics change with xzr/wzr.\n"); continue; } |