From f74a4298163a7d0b500c7f7a818829c153dc942e Mon Sep 17 00:00:00 2001 From: Bob Wilson Date: Sat, 30 Oct 2010 00:54:37 +0000 Subject: [PATCH] Overhaul memory barriers in the ARM backend. Radar 8601999. There were a number of issues to fix up here: * The "device" argument of the llvm.memory.barrier intrinsic should be used to distinguish the "Full System" domain from the "Inner Shareable" domain. It has nothing to do with using DMB vs. DSB instructions. * The compiler should never need to emit DSB instructions. Remove the ARMISD::SYNCBARRIER node and also remove the instruction patterns for DSB. * Merge the separate DMB/DSB instructions for options only used for the disassembler with the default DMB/DSB instructions. Add the default "full system" option ARM_MB::SY to the ARM_MB::MemBOpt enum. * Add a separate ARMISD::MEMBARRIER_MCR node for subtargets that implement a data memory barrier using the MCR instruction. * Fix up encodings for these instructions (except MCR). I also updated the tests and added a few new ones to check for DMB options that were not currently being exercised. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@117756 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/ARM/ARMBaseInfo.h | 2 + lib/Target/ARM/ARMISelLowering.cpp | 32 +++++++---- lib/Target/ARM/ARMISelLowering.h | 4 +- lib/Target/ARM/ARMInstrInfo.td | 73 +++++++---------------- lib/Target/ARM/ARMInstrThumb2.td | 83 ++++++--------------------- test/CodeGen/Thumb/barrier.ll | 10 ++-- test/CodeGen/Thumb2/thumb2-barrier.ll | 32 ++++++++--- 7 files changed, 90 insertions(+), 146 deletions(-) diff --git a/lib/Target/ARM/ARMBaseInfo.h b/lib/Target/ARM/ARMBaseInfo.h index 2da7be25a13..63e8f9655db 100644 --- a/lib/Target/ARM/ARMBaseInfo.h +++ b/lib/Target/ARM/ARMBaseInfo.h @@ -101,6 +101,7 @@ namespace ARM_MB { // The Memory Barrier Option constants map directly to the 4-bit encoding of // the option field for memory barrier operations. enum MemBOpt { + SY = 15, ST = 14, ISH = 11, ISHST = 10, @@ -113,6 +114,7 @@ namespace ARM_MB { inline static const char *MemBOptToString(unsigned val) { switch (val) { default: llvm_unreachable("Unknown memory operation"); + case SY: return "sy"; case ST: return "st"; case ISH: return "ish"; case ISHST: return "ishst"; diff --git a/lib/Target/ARM/ARMISelLowering.cpp b/lib/Target/ARM/ARMISelLowering.cpp index 810335b4c57..d086093597a 100644 --- a/lib/Target/ARM/ARMISelLowering.cpp +++ b/lib/Target/ARM/ARMISelLowering.cpp @@ -766,7 +766,7 @@ const char *ARMTargetLowering::getTargetNodeName(unsigned Opcode) const { case ARMISD::DYN_ALLOC: return "ARMISD::DYN_ALLOC"; case ARMISD::MEMBARRIER: return "ARMISD::MEMBARRIER"; - case ARMISD::SYNCBARRIER: return "ARMISD::SYNCBARRIER"; + case ARMISD::MEMBARRIER_MCR: return "ARMISD::MEMBARRIER_MCR"; case ARMISD::VCEQ: return "ARMISD::VCEQ"; case ARMISD::VCGE: return "ARMISD::VCGE"; @@ -2026,21 +2026,29 @@ ARMTargetLowering::LowerINTRINSIC_WO_CHAIN(SDValue Op, SelectionDAG &DAG, static SDValue LowerMEMBARRIER(SDValue Op, SelectionDAG &DAG, const ARMSubtarget *Subtarget) { DebugLoc dl = Op.getDebugLoc(); - SDValue Op5 = Op.getOperand(5); - unsigned isDeviceBarrier = cast(Op5)->getZExtValue(); - // Some subtargets which have dmb and dsb instructions can handle barriers - // directly. Some ARMv6 cpus can support them with the help of mcr - // instruction. Thumb1 and pre-v6 ARM mode use a libcall instead and should - // never get here. - unsigned Opc = isDeviceBarrier ? ARMISD::SYNCBARRIER : ARMISD::MEMBARRIER; - if (Subtarget->hasDataBarrier()) - return DAG.getNode(Opc, dl, MVT::Other, Op.getOperand(0)); - else { + if (!Subtarget->hasDataBarrier()) { + // Some ARMv6 cpus can support data barriers with an mcr instruction. + // Thumb1 and pre-v6 ARM mode use a libcall instead and should never get + // here. assert(Subtarget->hasV6Ops() && !Subtarget->isThumb1Only() && "Unexpected ISD::MEMBARRIER encountered. Should be libcall!"); - return DAG.getNode(Opc, dl, MVT::Other, Op.getOperand(0), + return DAG.getNode(ARMISD::MEMBARRIER_MCR, dl, MVT::Other, Op.getOperand(0), DAG.getConstant(0, MVT::i32)); } + + SDValue Op5 = Op.getOperand(5); + bool isDeviceBarrier = cast(Op5)->getZExtValue() != 0; + unsigned isLL = cast(Op.getOperand(1))->getZExtValue(); + unsigned isLS = cast(Op.getOperand(2))->getZExtValue(); + bool isOnlyStoreBarrier = (isLL == 0 && isLS == 0); + + ARM_MB::MemBOpt DMBOpt; + if (isDeviceBarrier) + DMBOpt = isOnlyStoreBarrier ? ARM_MB::ST : ARM_MB::SY; + else + DMBOpt = isOnlyStoreBarrier ? ARM_MB::ISHST : ARM_MB::ISH; + return DAG.getNode(ARMISD::MEMBARRIER, dl, MVT::Other, Op.getOperand(0), + DAG.getConstant(DMBOpt, MVT::i32)); } static SDValue LowerVASTART(SDValue Op, SelectionDAG &DAG) { diff --git a/lib/Target/ARM/ARMISelLowering.h b/lib/Target/ARM/ARMISelLowering.h index bf3553becc8..bd2fa8eb47e 100644 --- a/lib/Target/ARM/ARMISelLowering.h +++ b/lib/Target/ARM/ARMISelLowering.h @@ -81,8 +81,8 @@ namespace llvm { DYN_ALLOC, // Dynamic allocation on the stack. - MEMBARRIER, // Memory barrier - SYNCBARRIER, // Memory sync barrier + MEMBARRIER, // Memory barrier (DMB) + MEMBARRIER_MCR, // Memory barrier (MCR) VCEQ, // Vector compare equal. VCGE, // Vector compare greater than or equal. diff --git a/lib/Target/ARM/ARMInstrInfo.td b/lib/Target/ARM/ARMInstrInfo.td index 4497a92cceb..3bf601ef74f 100644 --- a/lib/Target/ARM/ARMInstrInfo.td +++ b/lib/Target/ARM/ARMInstrInfo.td @@ -60,10 +60,7 @@ def SDT_ARMEH_SJLJ_Longjmp: SDTypeProfile<0, 2, [SDTCisPtrTy<0>, SDTCisInt<1>]>; def SDT_ARMEH_SJLJ_DispatchSetup: SDTypeProfile<0, 1, [SDTCisPtrTy<0>]>; -def SDT_ARMMEMBARRIER : SDTypeProfile<0, 0, []>; -def SDT_ARMSYNCBARRIER : SDTypeProfile<0, 0, []>; -def SDT_ARMMEMBARRIERMCR : SDTypeProfile<0, 1, [SDTCisInt<0>]>; -def SDT_ARMSYNCBARRIERMCR : SDTypeProfile<0, 1, [SDTCisInt<0>]>; +def SDT_ARMMEMBARRIER : SDTypeProfile<0, 1, [SDTCisInt<0>]>; def SDT_ARMTCRET : SDTypeProfile<0, 1, [SDTCisPtrTy<0>]>; @@ -131,11 +128,7 @@ def ARMeh_sjlj_dispatchsetup: SDNode<"ARMISD::EH_SJLJ_DISPATCHSETUP", def ARMMemBarrier : SDNode<"ARMISD::MEMBARRIER", SDT_ARMMEMBARRIER, [SDNPHasChain]>; -def ARMSyncBarrier : SDNode<"ARMISD::SYNCBARRIER", SDT_ARMMEMBARRIER, - [SDNPHasChain]>; -def ARMMemBarrierMCR : SDNode<"ARMISD::MEMBARRIER", SDT_ARMMEMBARRIERMCR, - [SDNPHasChain]>; -def ARMSyncBarrierMCR : SDNode<"ARMISD::SYNCBARRIER", SDT_ARMMEMBARRIERMCR, +def ARMMemBarrierMCR : SDNode<"ARMISD::MEMBARRIER_MCR", SDT_ARMMEMBARRIER, [SDNPHasChain]>; def ARMrbit : SDNode<"ARMISD::RBIT", SDTIntUnaryOp>; @@ -2888,64 +2881,40 @@ def MOVCCi : AI1<0b1101, (outs GPR:$Rd), // Atomic operations intrinsics // -// memory barriers protect the atomic sequences -let hasSideEffects = 1 in { -def DMBsy : AInoP<(outs), (ins), MiscFrm, NoItinerary, "dmb", "", - [(ARMMemBarrier)]>, Requires<[IsARM, HasDB]> { - let Inst{31-4} = 0xf57ff05; - // FIXME: add support for options other than a full system DMB - // See DMB disassembly-only variants below. - let Inst{3-0} = 0b1111; +def memb_opt : Operand { + let PrintMethod = "printMemBOption"; } -def DSBsy : AInoP<(outs), (ins), MiscFrm, NoItinerary, "dsb", "", - [(ARMSyncBarrier)]>, Requires<[IsARM, HasDB]> { - let Inst{31-4} = 0xf57ff04; - // FIXME: add support for options other than a full system DSB - // See DSB disassembly-only variants below. - let Inst{3-0} = 0b1111; +// memory barriers protect the atomic sequences +let hasSideEffects = 1 in { +def DMB : AInoP<(outs), (ins memb_opt:$opt), MiscFrm, NoItinerary, + "dmb", "\t$opt", [(ARMMemBarrier (i32 imm:$opt))]>, + Requires<[IsARM, HasDB]> { + bits<4> opt; + let Inst{31-4} = 0xf57ff05; + let Inst{3-0} = opt; } def DMB_MCR : AInoP<(outs), (ins GPR:$zero), MiscFrm, NoItinerary, "mcr", "\tp15, 0, $zero, c7, c10, 5", [(ARMMemBarrierMCR GPR:$zero)]>, Requires<[IsARM, HasV6]> { - // FIXME: add support for options other than a full system DMB - // FIXME: add encoding -} - -def DSB_MCR : AInoP<(outs), (ins GPR:$zero), MiscFrm, NoItinerary, - "mcr", "\tp15, 0, $zero, c7, c10, 4", - [(ARMSyncBarrierMCR GPR:$zero)]>, - Requires<[IsARM, HasV6]> { - // FIXME: add support for options other than a full system DSB // FIXME: add encoding } } -// Memory Barrier Operations Variants -- for disassembly only - -def memb_opt : Operand { - let PrintMethod = "printMemBOption"; +def DSB : AInoP<(outs), (ins memb_opt:$opt), MiscFrm, NoItinerary, + "dsb", "\t$opt", + [/* For disassembly only; pattern left blank */]>, + Requires<[IsARM, HasDB]> { + bits<4> opt; + let Inst{31-4} = 0xf57ff04; + let Inst{3-0} = opt; } -class AMBI op7_4, string opc> - : AInoP<(outs), (ins memb_opt:$opt), MiscFrm, NoItinerary, opc, "\t$opt", - [/* For disassembly only; pattern left blank */]>, - Requires<[IsARM, HasDB]> { - let Inst{31-8} = 0xf57ff0; - let Inst{7-4} = op7_4; -} - -// These DMB variants are for disassembly only. -def DMBvar : AMBI<0b0101, "dmb">; - -// These DSB variants are for disassembly only. -def DSBvar : AMBI<0b0100, "dsb">; - // ISB has only full system option -- for disassembly only -def ISBsy : AInoP<(outs), (ins), MiscFrm, NoItinerary, "isb", "", []>, - Requires<[IsARM, HasDB]> { +def ISB : AInoP<(outs), (ins), MiscFrm, NoItinerary, "isb", "", []>, + Requires<[IsARM, HasDB]> { let Inst{31-4} = 0xf57ff06; let Inst{3-0} = 0b1111; } diff --git a/lib/Target/ARM/ARMInstrThumb2.td b/lib/Target/ARM/ARMInstrThumb2.td index a24281d16c3..f11ccb31ed6 100644 --- a/lib/Target/ARM/ARMInstrThumb2.td +++ b/lib/Target/ARM/ARMInstrThumb2.td @@ -2268,78 +2268,29 @@ def t2MOVCCror : T2I_movcc_sh<0b11, (outs rGPR:$dst), // memory barriers protect the atomic sequences let hasSideEffects = 1 in { -def t2DMBsy : AInoP<(outs), (ins), ThumbFrm, NoItinerary, "dmb", "", - [(ARMMemBarrier)]>, Requires<[IsThumb, HasDB]> { - let Inst{31-4} = 0xF3BF8F5; - // FIXME: add support for options other than a full system DMB - let Inst{3-0} = 0b1111; -} - -def t2DSBsy : AInoP<(outs), (ins), ThumbFrm, NoItinerary, "dsb", "", - [(ARMSyncBarrier)]>, Requires<[IsThumb, HasDB]> { - let Inst{31-4} = 0xF3BF8F4; - // FIXME: add support for options other than a full system DSB - let Inst{3-0} = 0b1111; +def t2DMB : AInoP<(outs), (ins memb_opt:$opt), ThumbFrm, NoItinerary, + "dmb", "\t$opt", [(ARMMemBarrier (i32 imm:$opt))]>, + Requires<[IsThumb, HasDB]> { + bits<4> opt; + let Inst{31-4} = 0xf3bf8f5; + let Inst{3-0} = opt; } } -// Helper class for multiclass T2MemB -- for disassembly only -class T2I_memb - : T2I<(outs), (ins), NoItinerary, opc, asm, - [/* For disassembly only; pattern left blank */]>, - Requires<[IsThumb2, HasV7]> { - let Inst{31-20} = 0xf3b; - let Inst{15-14} = 0b10; - let Inst{12} = 0; +def t2DSB : AInoP<(outs), (ins memb_opt:$opt), ThumbFrm, NoItinerary, + "dsb", "\t$opt", + [/* For disassembly only; pattern left blank */]>, + Requires<[IsThumb, HasDB]> { + bits<4> opt; + let Inst{31-4} = 0xf3bf8f4; + let Inst{3-0} = opt; } -multiclass T2MemB op7_4, string opc> { - - def st : T2I_memb { - let Inst{7-4} = op7_4; - let Inst{3-0} = 0b1110; - } - - def ish : T2I_memb { - let Inst{7-4} = op7_4; - let Inst{3-0} = 0b1011; - } - - def ishst : T2I_memb { - let Inst{7-4} = op7_4; - let Inst{3-0} = 0b1010; - } - - def nsh : T2I_memb { - let Inst{7-4} = op7_4; - let Inst{3-0} = 0b0111; - } - - def nshst : T2I_memb { - let Inst{7-4} = op7_4; - let Inst{3-0} = 0b0110; - } - - def osh : T2I_memb { - let Inst{7-4} = op7_4; - let Inst{3-0} = 0b0011; - } - - def oshst : T2I_memb { - let Inst{7-4} = op7_4; - let Inst{3-0} = 0b0010; - } -} - -// These DMB variants are for disassembly only. -defm t2DMB : T2MemB<0b0101, "dmb">; - -// These DSB variants are for disassembly only. -defm t2DSB : T2MemB<0b0100, "dsb">; - // ISB has only full system option -- for disassembly only -def t2ISBsy : T2I_memb<"isb", ""> { - let Inst{7-4} = 0b0110; +def t2ISB : T2I<(outs), (ins), NoItinerary, "isb", "", + [/* For disassembly only; pattern left blank */]>, + Requires<[IsThumb2, HasV7]> { + let Inst{31-4} = 0xf3bf8f6; let Inst{3-0} = 0b1111; } diff --git a/test/CodeGen/Thumb/barrier.ll b/test/CodeGen/Thumb/barrier.ll index c611b865f67..cab658e79a1 100644 --- a/test/CodeGen/Thumb/barrier.ll +++ b/test/CodeGen/Thumb/barrier.ll @@ -1,15 +1,15 @@ ; RUN: llc < %s -mtriple=thumbv6-apple-darwin | FileCheck %s -check-prefix=V6 ; RUN: llc < %s -march=thumb -mattr=+v6m | FileCheck %s -check-prefix=V6M -declare void @llvm.memory.barrier( i1 , i1 , i1 , i1 , i1 ) +declare void @llvm.memory.barrier(i1 , i1 , i1 , i1 , i1) define void @t1() { ; V6: t1: ; V6: blx {{_*}}sync_synchronize ; V6M: t1: -; V6M: dsb - call void @llvm.memory.barrier( i1 false, i1 false, i1 false, i1 true, i1 true ) +; V6M: dmb st + call void @llvm.memory.barrier(i1 false, i1 false, i1 false, i1 true, i1 true) ret void } @@ -18,7 +18,7 @@ define void @t2() { ; V6: blx {{_*}}sync_synchronize ; V6M: t2: -; V6M: dmb - call void @llvm.memory.barrier( i1 false, i1 false, i1 false, i1 true, i1 false ) +; V6M: dmb ish + call void @llvm.memory.barrier(i1 true, i1 false, i1 false, i1 true, i1 false) ret void } diff --git a/test/CodeGen/Thumb2/thumb2-barrier.ll b/test/CodeGen/Thumb2/thumb2-barrier.ll index a54d09e6291..93ae7c428bd 100644 --- a/test/CodeGen/Thumb2/thumb2-barrier.ll +++ b/test/CodeGen/Thumb2/thumb2-barrier.ll @@ -1,17 +1,31 @@ ; RUN: llc < %s -march=thumb -mcpu=cortex-a8 | FileCheck %s -declare void @llvm.memory.barrier( i1 , i1 , i1 , i1 , i1 ) +declare void @llvm.memory.barrier(i1 , i1 , i1 , i1 , i1) -define void @t1() { -; CHECK: t1: -; CHECK: dsb - call void @llvm.memory.barrier( i1 false, i1 false, i1 false, i1 true, i1 true ) +define void @t_st() { +; CHECK: t_st: +; CHECK: dmb st + call void @llvm.memory.barrier(i1 false, i1 false, i1 false, i1 true, i1 true) ret void } -define void @t2() { -; CHECK: t2: -; CHECK: dmb - call void @llvm.memory.barrier( i1 false, i1 false, i1 false, i1 true, i1 false ) +define void @t_sy() { +; CHECK: t_sy: +; CHECK: dmb sy + call void @llvm.memory.barrier(i1 true, i1 false, i1 false, i1 true, i1 true) + ret void +} + +define void @t_ishst() { +; CHECK: t_ishst: +; CHECK: dmb ishst + call void @llvm.memory.barrier(i1 false, i1 false, i1 false, i1 true, i1 false) + ret void +} + +define void @t_ish() { +; CHECK: t_ish: +; CHECK: dmb ish + call void @llvm.memory.barrier(i1 true, i1 false, i1 false, i1 true, i1 false) ret void }