Port memory barriers intrinsics to AArch64

[kongyi]

Summary

Memory barrier __builtin_arm_[dmb, dsb, isb] intrinsics are required to implement their corresponding ACLE and MSVC intrinsics.

This patch ports ARM dmb, dsb, isb intrinsic to AArch64. It relies on D4516.

[t.p.northover]

Hi Yi,

Are you sure this is going to interact well with D4516? The builtins you're adding are in the "__builtin_arm" space but that's (correctly, IMO) changing the prefix to "aarch64". Either way, we probably want a Clang-side test for that GCCBuiltin too.

One other thing I noticed was some extra complexity that shouldn't be needed (see end).

Cheers.

Tim.

lib/Target/AArch64/AArch64InstrFormats.td

546 ↗

(On Diff #11442)

Something weird is going on here. You're adding a ParserMatchClass but not using this operand in any assembly context so it's almost certainly ignored. You probably only need the "ImmLeaf" part of this definition.

[kongyi]

Updated AArchIntrFormats.td according to Tim's comments.

[compnerd]

lib/Target/AArch64/AArch64InstrFormats.td

830 ↗

(On Diff #11447)

I guess that I should've done this when I was changing the other System instructions.

test/CodeGen/AArch64/intrinsics-memory-barrier.ll

1 ↗

(On Diff #11447)

Can you use aarch64-eabi please?

[kongyi]

Changed target triple of the test case to aarch64-eabi.

[t.p.northover]

Hi Yi,

This looks fine to me too.

Cheers.

Tim.

[kongyi]

Closed by commit rL213247 (authored by @kongyi).

llvm/trunk/include/llvm/IR/IntrinsicsAArch64.td

 def int_aarch64_hint : Intrinsic<[], [llvm_i32_ty]>;
 
 //===----------------------------------------------------------------------===//
 // RBIT
 
 def int_aarch64_rbit : Intrinsic<[llvm_anyint_ty], [LLVMMatchType<0>],
                                  [IntrNoMem]>;
 
+//===----------------------------------------------------------------------===//
+// Data Barrier Instructions
+
+def int_aarch64_dmb : GCCBuiltin<"__builtin_arm_dmb">, Intrinsic<[], [llvm_i32_ty]>;
+def int_aarch64_dsb : GCCBuiltin<"__builtin_arm_dsb">, Intrinsic<[], [llvm_i32_ty]>;
+def int_aarch64_isb : GCCBuiltin<"__builtin_arm_isb">, Intrinsic<[], [llvm_i32_ty]>;
+
 }
 
 //===----------------------------------------------------------------------===//
 // Advanced SIMD (NEON)
 
 let TargetPrefix = "aarch64" in {  // All intrinsics start with "llvm.aarch64.".
   class AdvSIMD_2Scalar_Float_Intrinsic
     : Intrinsic<[llvm_anyfloat_ty], [LLVMMatchType<0>, LLVMMatchType<0>],

llvm/trunk/lib/Target/AArch64/AArch64InstrFormats.td

 
 // imm0_7 predicate - True if the immediate is in the range [0,7]
 def imm0_7 : Operand<i64>, ImmLeaf<i64, [{
   return ((uint64_t)Imm) < 8;
 }]> {
   let ParserMatchClass = Imm0_7Operand;
 }
 
+// imm32_0_15 predicate - True if the 32-bit immediate is in the range [0,15]
+def imm32_0_15 : Operand<i32>, ImmLeaf<i32, [{
+  return ((uint32_t)Imm) < 16;
+}]>;
+
 // An arithmetic shifter operand:
 //  {7-6} - shift type: 00 = lsl, 01 = lsr, 10 = asr
 //  {5-0} - imm6
 class arith_shift<ValueType Ty, int width> : Operand<Ty> {
   let PrintMethod = "printShifter";
   let ParserMatchClass = !cast<AsmOperandClass>(
                          "ArithmeticShifterOperand" # width);
 }

 def BarrierAsmOperand : AsmOperandClass {
   let Name = "Barrier";
   let ParserMethod = "tryParseBarrierOperand";
 }
 def barrier_op : Operand<i32> {
   let PrintMethod = "printBarrierOption";
   let ParserMatchClass = BarrierAsmOperand;
 }
-class CRmSystemI<Operand crmtype, bits<3> opc, string asm>
-    : SimpleSystemI<0, (ins crmtype:$CRm), asm, "\t$CRm">,
+class CRmSystemI<Operand crmtype, bits<3> opc, string asm,
+                 list<dag> pattern = []>
+    : SimpleSystemI<0, (ins crmtype:$CRm), asm, "\t$CRm", pattern>,
       Sched<[WriteBarrier]> {
   bits<4> CRm;
   let Inst{20-12} = 0b000110011;
   let Inst{11-8} = CRm;
   let Inst{7-5} = opc;
 }
 
 // MRS/MSR system instructions. These have different operand classes because

llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td

 def HINT : HintI<"hint">;
 def : InstAlias<"nop",  (HINT 0b000)>;
 def : InstAlias<"yield",(HINT 0b001)>;
 def : InstAlias<"wfe",  (HINT 0b010)>;
 def : InstAlias<"wfi",  (HINT 0b011)>;
 def : InstAlias<"sev",  (HINT 0b100)>;
 def : InstAlias<"sevl", (HINT 0b101)>;
 
-  // As far as LLVM is concerned this writes to the system's exclusive monitors.
 let mayLoad = 1, mayStore = 1 in
 def CLREX : CRmSystemI<imm0_15, 0b010, "clrex">;
 
-def DMB   : CRmSystemI<barrier_op, 0b101, "dmb">;
-def DSB   : CRmSystemI<barrier_op, 0b100, "dsb">;
-def ISB   : CRmSystemI<barrier_op, 0b110, "isb">;
+// NOTE: ideally, this would have mayStore = 0, mayLoad = 0, but we cannot
+// model patterns with sufficiently fine granularity.
+let mayLoad = ?, mayStore = ? in {
+def DMB   : CRmSystemI<barrier_op, 0b101, "dmb",
+                       [(int_aarch64_dmb (i32 imm32_0_15:$CRm))]>;
+
+def DSB   : CRmSystemI<barrier_op, 0b100, "dsb",
+                       [(int_aarch64_dsb (i32 imm32_0_15:$CRm))]>;
+
+def ISB   : CRmSystemI<barrier_op, 0b110, "isb",
+                       [(int_aarch64_isb (i32 imm32_0_15:$CRm))]>;
+}
+
 def : InstAlias<"clrex", (CLREX 0xf)>;
 def : InstAlias<"isb", (ISB 0xf)>;
 
 def MRS    : MRSI;
 def MSR    : MSRI;
 def MSRpstate: MSRpstateI;
 
 // The thread pointer (on Linux, at least, where this has been implemented) is

llvm/trunk/test/CodeGen/AArch64/intrinsics-memory-barrier.ll

+; RUN: llc < %s -mtriple=aarch64-eabi -O=3 | FileCheck %s
+
+define void @test() {
+  ; CHECK: dmb sy
+  call void @llvm.aarch64.dmb(i32 15)
+  ; CHECK: dmb osh
+  call void @llvm.aarch64.dmb(i32 3)
+  ; CHECK: dsb sy
+  call void @llvm.aarch64.dsb(i32 15)
+  ; CHECK: dsb ishld
+  call void @llvm.aarch64.dsb(i32 9)
+  ; CHECK: isb
+  call void @llvm.aarch64.isb(i32 15)
+  ret void
+}
+
+; Important point is that the compiler should not reorder memory access
+; instructions around DMB.
+; Failure to do so, two STRs will collapse into one STP.
+define void @test_dmb_reordering(i32 %a, i32 %b, i32* %d) {
+  store i32 %a, i32* %d              ; CHECK: str {{w[0-9]+}}, [{{x[0-9]+}}]
+
+  call void @llvm.aarch64.dmb(i32 15); CHECK: dmb sy
+
+  %d1 = getelementptr i32* %d, i64 1
+  store i32 %b, i32* %d1             ; CHECK: str {{w[0-9]+}}, [{{x[0-9]+}}, #4]
+
+  ret void
+}
+
+; Similarly for DSB.
+define void @test_dsb_reordering(i32 %a, i32 %b, i32* %d) {
+  store i32 %a, i32* %d              ; CHECK: str {{w[0-9]+}}, [{{x[0-9]+}}]
+
+  call void @llvm.aarch64.dsb(i32 15); CHECK: dsb sy
+
+  %d1 = getelementptr i32* %d, i64 1
+  store i32 %b, i32* %d1             ; CHECK: str {{w[0-9]+}}, [{{x[0-9]+}}, #4]
+
+  ret void
+}
+
+; And ISB.
+define void @test_isb_reordering(i32 %a, i32 %b, i32* %d) {
+  store i32 %a, i32* %d              ; CHECK: str {{w[0-9]+}}, [{{x[0-9]+}}]
+
+  call void @llvm.aarch64.isb(i32 15); CHECK: isb
+
+  %d1 = getelementptr i32* %d, i64 1
+  store i32 %b, i32* %d1             ; CHECK: str {{w[0-9]+}}, [{{x[0-9]+}}, #4]
+
+  ret void
+}
+
+declare void @llvm.aarch64.dmb(i32)
+declare void @llvm.aarch64.dsb(i32)
+declare void @llvm.aarch64.isb(i32)