[ARM][AArch64] Switch to generic MEMBARRIER node

[reames]

Summary

This change switches both targets from using target specific CompilerBarrier nodes to the recently introduced generic MEMBARRIER instruction.

A couple things to call out.

First, this changes the assembly comment printed. I'm not sure this matters, but if it does, we can simply drop this patch. This is a minor clean up at best.

Second, the ordering operand on the target instruction appears to be unused. We could easily add ordering to the generic instruction, but since we don't seem to have a motivating case in tree, I simply dropped the ordering when selecting to the generic instruction.

[lenary]

If the ordering is not used, I don't mind us removing it.

It might be worth documenting that MEMBARRIER should maybe be treated as a seq_cst singlescope fence though? This would mean it has an ordering, even if no ordering requirement.

llvm/lib/Target/AArch64/AArch64InstrAtomics.td

 //=- AArch64InstrAtomics.td - AArch64 Atomic codegen support -*- tablegen -*-=//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // AArch64 Atomic operand code-gen constructs.
 //
 //===----------------------------------------------------------------------===//
 
 //===----------------------------------
 // Atomic fences
 //===----------------------------------
-let AddedComplexity = 15, Size = 0 in
-def CompilerBarrier : Pseudo<(outs), (ins i32imm:$ordering),
-                             [(atomic_fence timm:$ordering, 0)]>, Sched<[]>;
+let AddedComplexity = 15 in
+def : Pat<(atomic_fence (timm), 0), (MEMBARRIER)>;
 def : Pat<(atomic_fence (i64 4), (timm)), (DMB (i32 0x9))>;
 def : Pat<(atomic_fence (timm), (timm)), (DMB (i32 0xb))>;
 
 //===----------------------------------
 // Atomic loads
 //===----------------------------------
 
 // When they're actually atomic, only one addressing mode (GPR64sp) is

llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

case TargetOpcode::G_OR: {
     int64_t Imms = Size - ShiftImm - 1;
     unsigned Opc = Size == 32 ? AArch64::BFMWri : AArch64::BFMXri;
     emitInstr(Opc, {Dst}, {MaskSrc, ShiftSrc, Immr, Imms}, MIB);
     I.eraseFromParent();
     return true;
   }
   case TargetOpcode::G_FENCE: {
     if (I.getOperand(1).getImm() == 0)
-      BuildMI(MBB, I, MIMetadata(I), TII.get(AArch64::CompilerBarrier))
-          .addImm(I.getOperand(0).getImm());
+      BuildMI(MBB, I, MIMetadata(I), TII.get(TargetOpcode::MEMBARRIER));
     else
       BuildMI(MBB, I, MIMetadata(I), TII.get(AArch64::DMB))
           .addImm(I.getOperand(0).getImm() == 4 ? 0x9 : 0xb);
     I.eraseFromParent();
     return true;
   }
   default:
     return false;

llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.cpp

if (!AArch64_AM::isAnyMOVWMovAlias(Value, RegWidth)) {
       O << "\tmov\t";
       printRegName(O, MI->getOperand(0).getReg());
       O << ", " << markup("<imm:") << "#"
         << formatImm(SignExtend64(Value, RegWidth)) << markup(">");
       return;
     }
   }
 
-  if (Opcode == AArch64::CompilerBarrier) {
-    O << '\t' << MAI.getCommentString() << " COMPILER BARRIER";
-    printAnnotation(O, Annot);
-    return;
-  }
-
   if (Opcode == AArch64::SPACE) {
     O << '\t' << MAI.getCommentString() << " SPACE "
       << MI->getOperand(1).getImm();
     printAnnotation(O, Annot);
     return;
   }
 
   // Instruction TSB is specified as a one operand instruction, but 'csync' is

llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp

auto Reloc = STI.getTargetTriple().getEnvironment() == Triple::GNUILP32
                      ? ELF::R_AARCH64_P32_TLSDESC_CALL
                      : ELF::R_AARCH64_TLSDESC_CALL;
     Fixups.push_back(
         MCFixup::create(0, MI.getOperand(0).getExpr(),
                         MCFixupKind(FirstLiteralRelocationKind + Reloc)));
     return;
   }
 
-  if (MI.getOpcode() == AArch64::CompilerBarrier ||
-      MI.getOpcode() == AArch64::SPACE) {
-    // CompilerBarrier just prevents the compiler from reordering accesses, and
+  if (MI.getOpcode() == AArch64::SPACE) {
     // SPACE just increases basic block size, in both cases no actual code.
     return;
   }
 
   uint64_t Binary = getBinaryCodeForInstr(MI, Fixups, STI);
   support::endian::write<uint32_t>(OS, Binary, support::little);
   ++MCNumEmitted; // Keep track of the # of mi's emitted.
 }

llvm/lib/Target/ARM/ARMInstrInfo.td

def CMP_SWAP_32 : PseudoInst<(outs GPR:$Rd, GPR:$temp),
                              (ins GPR:$addr, GPR:$desired, GPR:$new),
                              NoItinerary, []>, Sched<[]>;
 
 def CMP_SWAP_64 : PseudoInst<(outs GPRPair:$Rd, GPR:$temp),
                              (ins GPR:$addr, GPRPair:$desired, GPRPair:$new),
                              NoItinerary, []>, Sched<[]>;
 }
 
-def CompilerBarrier : PseudoInst<(outs), (ins i32imm:$ordering), NoItinerary,
-                                 [(atomic_fence timm:$ordering, 0)]> {
-  let hasSideEffects = 1;
-  let Size = 0;
-  let AsmString = "@ COMPILER BARRIER";
-  let hasNoSchedulingInfo = 1;
-}
+def : Pat<(atomic_fence (timm), 0), (MEMBARRIER)>;
 
 //===----------------------------------------------------------------------===//
 // Instructions used for emitting unwind opcodes on Windows.
 //===----------------------------------------------------------------------===//
 let isPseudo = 1 in {
   def SEH_StackAlloc : PseudoInst<(outs), (ins i32imm:$size, i32imm:$wide), NoItinerary, []>, Sched<[]>;
   def SEH_SaveRegs : PseudoInst<(outs), (ins i32imm:$mask, i32imm:$wide), NoItinerary, []>, Sched<[]>;
   let isTerminator = 1 in

llvm/lib/Target/ARM/ARMScheduleA57.td

 
 // To have "CompleteModel = 1", support of pseudos and special instructions
 def : InstRW<[WriteNoop], (instregex "(t)?BKPT$", "(t2)?CDP(2)?$",
   "(t2)?CLREX$", "CONSTPOOL_ENTRY$", "COPY_STRUCT_BYVAL_I32$",
   "(t2)?CPS[123]p$", "(t2)?DBG$", "(t2)?DMB$", "(t2)?DSB$", "ERET$",
   "(t2|t)?HINT$", "(t)?HLT$", "(t2)?HVC$", "(t2)?ISB$", "ITasm$",
   "(t2)?RFE(DA|DB|IA|IB)", "(t)?SETEND", "(t2)?SETPAN", "(t2)?SMC", "SPACE",
   "(t2)?SRS(DA|DB|IA|IB)", "SWP(B)?", "t?TRAP", "(t2|t)?UDF$", "t2DCPS", "t2SG",
-  "t2TT", "tCPS", "CMP_SWAP", "t?SVC", "t2IT", "CompilerBarrier",
-  "t__brkdiv0")>;
+  "t2TT", "tCPS", "CMP_SWAP", "t?SVC", "t2IT", "t__brkdiv0")>;
 
 def : InstRW<[WriteNoop], (instregex "VMRS", "VMSR", "FMSTAT")>;
 
 // Specific memory instrs
 def : InstRW<[WriteNoop, WriteNoop], (instregex "(t2)?LDA", "(t2)?LDC", "(t2)?STC",
   "(t2)?STL", "(t2)?LDREX", "(t2)?STREX", "MEMCPY")>;
 
 // coprocessor moves

llvm/test/CodeGen/AArch64/fence-singlethread.ll

 ; RUN: llc -mtriple=aarch64-linux-gnu %s -o - | FileCheck %s --check-prefix=LINUX
 ; RUN: llc -mtriple=aarch64-apple-ios %s -o - | FileCheck %s --check-prefix=IOS
 ; RUN: llc -mtriple=aarch64-apple-ios %s -o - -global-isel | FileCheck %s --check-prefix=IOS
 ; RUN: llc -mtriple=aarch64-linux-gnueabihf %s -filetype=obj -o %t
 ; RUN: llvm-objdump -d %t | FileCheck %s --check-prefix=OBJ
 
 ; OBJ-NOT: dmb
 
 define void @fence_singlethread() {
 ; LINUX-LABEL: fence_singlethread:
 ; LINUX-NOT: dmb
-; LINUX: // COMPILER BARRIER
+; LINUX: //MEMBARRIER
 ; LINUX-NOT: dmb
 
 ; IOS-LABEL: fence_singlethread:
 ; IOS-NOT: dmb
-; IOS: ; COMPILER BARRIER
+; IOS: ;MEMBARRIER
 ; IOS-NOT: dmb
 
   fence syncscope("singlethread") seq_cst
   ret void
 }

llvm/test/CodeGen/ARM/fence-singlethread.ll

 ; RUN: llc -mtriple=thumbv7-linux-gnueabihf %s -o - | FileCheck %s
 ; RUN: llc -mtriple=thumbv7-apple-ios %s -o - | FileCheck %s
 ; RUN: llc -mtriple=thumbv7-linux-gnueabihf %s -filetype=obj -o %t
 ; RUN: llvm-objdump -d %t | FileCheck %s --check-prefix=OBJ
 
 ; OBJ-NOT: dmb
 
 define void @fence_singlethread() {
 ; CHECK-LABEL: fence_singlethread:
 ; CHECK-NOT: dmb
-; CHECK: @ COMPILER BARRIER
+; CHECK: @MEMBARRIER
 ; CHECK-NOT: dmb
 
   fence syncscope("singlethread") seq_cst
   ret void
 }