[llvm] eb44226 - [CodeGen] Introduce a generic MEMBARRIER instruction [mostly-nfc]
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 11 07:26:53 PST 2023
Author: Philip Reames
Date: 2023-01-11T07:26:27-08:00
New Revision: eb44226986fcbeb4325e5f668e5646e9646958bc
URL: https://github.com/llvm/llvm-project/commit/eb44226986fcbeb4325e5f668e5646e9646958bc
DIFF: https://github.com/llvm/llvm-project/commit/eb44226986fcbeb4325e5f668e5646e9646958bc.diff
LOG: [CodeGen] Introduce a generic MEMBARRIER instruction [mostly-nfc]
This is a follow up to D141317 which extends the common code to include a target independent pseudo instruction. This is an alternative to (subset of) D92842 which tries to be as close to NFC as possible.
A couple things to call out.
* The test change in X86 is because we loose the scheduling information on the instruction. However, I think this was actually a bug in x86 since no instruction was emitted for a MEMBARRIER. Concluding that a meta instruction has latency just seems wrong?
* I intentionally left some parts of D92842 out. Specifically, several of the changes in the X86 code (data independence and outlining) appear functional, and likely worthy of their own review. Additionally, I'm not handling ARM/AArch64 at all. Those targets need the ordering whereas none of the others do. I want to get this in and tested before retrofitting in ordering to support those targets.
Differential Revision: https://reviews.llvm.org/D141408
Added:
Modified:
llvm/include/llvm/CodeGen/SelectionDAGISel.h
llvm/include/llvm/Support/TargetOpcodes.def
llvm/include/llvm/Target/Target.td
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
llvm/lib/Target/RISCV/RISCVInstrInfo.td
llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp
llvm/lib/Target/SystemZ/SystemZInstrInfo.td
llvm/lib/Target/SystemZ/SystemZLongBranch.cpp
llvm/lib/Target/VE/VEInstrInfo.td
llvm/lib/Target/X86/X86InstrCompiler.td
llvm/lib/Target/X86/X86MCInstLower.cpp
llvm/lib/Target/XCore/XCoreInstrInfo.td
llvm/test/CodeGen/X86/atomic-idempotent.ll
llvm/unittests/MIR/MachineMetadata.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/SelectionDAGISel.h b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
index 3730ab026c664..d690a186f3695 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGISel.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
@@ -321,6 +321,7 @@ class SelectionDAGISel : public MachineFunctionPass {
void Select_FREEZE(SDNode *N);
void Select_ARITH_FENCE(SDNode *N);
+ void Select_MEMBARRIER(SDNode *N);
void pushStackMapLiveVariable(SmallVectorImpl<SDValue> &Ops, SDValue Operand,
SDLoc DL);
diff --git a/llvm/include/llvm/Support/TargetOpcodes.def b/llvm/include/llvm/Support/TargetOpcodes.def
index f3a1b638ccc59..1a95081746daf 100644
--- a/llvm/include/llvm/Support/TargetOpcodes.def
+++ b/llvm/include/llvm/Support/TargetOpcodes.def
@@ -222,6 +222,10 @@ HANDLE_TARGET_OPCODE(PATCHABLE_TYPED_EVENT_CALL)
HANDLE_TARGET_OPCODE(ICALL_BRANCH_FUNNEL)
+// This is a fence with the singlethread scope. It represents a compiler memory
+// barrier, but does not correspond to any generated instruction.
+HANDLE_TARGET_OPCODE(MEMBARRIER)
+
/// The following generic opcodes are not supposed to appear after ISel.
/// This is something we might want to relax, but for now, this is convenient
/// to produce diagnostics.
diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index 5e3311323eafe..dccb1469ee9bc 100644
--- a/llvm/include/llvm/Target/Target.td
+++ b/llvm/include/llvm/Target/Target.td
@@ -1432,6 +1432,14 @@ def ICALL_BRANCH_FUNNEL : StandardPseudoInstruction {
let AsmString = "";
let hasSideEffects = true;
}
+def MEMBARRIER : StandardPseudoInstruction {
+ let OutOperandList = (outs);
+ let InOperandList = (ins);
+ let AsmString = "";
+ let hasSideEffects = true;
+ let Size = 0;
+ let isMeta = true;
+}
// Generic opcodes used in GlobalISel.
include "llvm/Target/GenericOpcodes.td"
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 11e6de92434dc..5375c14648210 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1653,6 +1653,9 @@ void AsmPrinter::emitFunctionBody() {
if (isVerbose())
OutStreamer->emitRawComment("ARITH_FENCE");
break;
+ case TargetOpcode::MEMBARRIER:
+ OutStreamer->emitRawComment("MEMBARRIER");
+ break;
default:
emitInstruction(&MI);
if (CanDoExtraAnalysis) {
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index d8be270ce8475..a01f85504ca45 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -2245,6 +2245,11 @@ void SelectionDAGISel::Select_ARITH_FENCE(SDNode *N) {
N->getOperand(0));
}
+void SelectionDAGISel::Select_MEMBARRIER(SDNode *N) {
+ CurDAG->SelectNodeTo(N, TargetOpcode::MEMBARRIER, N->getValueType(0),
+ N->getOperand(0));
+}
+
void SelectionDAGISel::pushStackMapLiveVariable(SmallVectorImpl<SDValue> &Ops,
SDValue OpVal, SDLoc DL) {
SDNode *OpNode = OpVal.getNode();
@@ -2899,6 +2904,9 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
case ISD::ARITH_FENCE:
Select_ARITH_FENCE(NodeToMatch);
return;
+ case ISD::MEMBARRIER:
+ Select_MEMBARRIER(NodeToMatch);
+ return;
case ISD::STACKMAP:
Select_STACKMAP(NodeToMatch);
return;
diff --git a/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp b/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
index 3bafbccac9d59..5c36a917caeaa 100644
--- a/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ b/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -116,9 +116,6 @@ void RISCVAsmPrinter::emitInstruction(const MachineInstr *MI) {
case RISCV::HWASAN_CHECK_MEMACCESS_SHORTGRANULES:
LowerHWASAN_CHECK_MEMACCESS(*MI);
return;
- case RISCV::PseudoMemBarrier:
- OutStreamer->emitRawComment("MEMBARRIER");
- return;
}
MCInst TmpInst;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index e46982eb3da8c..c699a94943d82 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -1843,9 +1843,6 @@ def : Pat<(binop_allwusers<add> GPR:$rs1, (AddiPair:$rs2)),
(AddiPairImmSmall AddiPair:$rs2))>;
}
-let hasSideEffects = 1, isMeta = 1 in
-def PseudoMemBarrier : Pseudo<(outs), (ins), [(membarrier)]>;
-
//===----------------------------------------------------------------------===//
// Standard extensions
//===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp b/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp
index 20c666a643407..3e63f17c6518d 100644
--- a/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp
@@ -530,11 +530,6 @@ void SystemZAsmPrinter::emitInstruction(const MachineInstr *MI) {
.addImm(15).addReg(SystemZ::R0D);
break;
- // Emit nothing here but a comment if we can.
- case SystemZ::MemBarrier:
- OutStreamer->emitRawComment("MEMBARRIER");
- return;
-
// We want to emit "j .+2" for traps, jumping to the relative immediate field
// of the jump instruction, which is an illegal instruction. We cannot emit a
// "." symbol, so create and emit a temp label before the instruction and use
diff --git a/llvm/lib/Target/SystemZ/SystemZInstrInfo.td b/llvm/lib/Target/SystemZ/SystemZInstrInfo.td
index 59b3d54effa57..c53cb7cadadb9 100644
--- a/llvm/lib/Target/SystemZ/SystemZInstrInfo.td
+++ b/llvm/lib/Target/SystemZ/SystemZInstrInfo.td
@@ -1716,10 +1716,6 @@ let Predicates = [FeatureExecutionHint], hasSideEffects = 1 in {
let hasSideEffects = 1 in
def Serialize : Alias<2, (outs), (ins), []>;
-// A pseudo instruction that serves as a compiler barrier.
-let hasSideEffects = 1, hasNoSchedulingInfo = 1 in
-def MemBarrier : Pseudo<(outs), (ins), [(membarrier)]>;
-
let Predicates = [FeatureInterlockedAccess1], Defs = [CC] in {
def LAA : LoadAndOpRSY<"laa", 0xEBF8, atomic_load_add_32, GR32>;
def LAAG : LoadAndOpRSY<"laag", 0xEBE8, atomic_load_add_64, GR64>;
diff --git a/llvm/lib/Target/SystemZ/SystemZLongBranch.cpp b/llvm/lib/Target/SystemZ/SystemZLongBranch.cpp
index d53693154d404..632218cc61eef 100644
--- a/llvm/lib/Target/SystemZ/SystemZLongBranch.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZLongBranch.cpp
@@ -217,7 +217,7 @@ static unsigned getInstSizeInBytes(const MachineInstr &MI,
assert((Size ||
// These do not have a size:
MI.isDebugOrPseudoInstr() || MI.isPosition() || MI.isKill() ||
- MI.isImplicitDef() || MI.getOpcode() == SystemZ::MemBarrier ||
+ MI.isImplicitDef() || MI.getOpcode() == TargetOpcode::MEMBARRIER ||
// These have a size that may be zero:
MI.isInlineAsm() || MI.getOpcode() == SystemZ::STACKMAP ||
MI.getOpcode() == SystemZ::PATCHPOINT) &&
diff --git a/llvm/lib/Target/VE/VEInstrInfo.td b/llvm/lib/Target/VE/VEInstrInfo.td
index ef8b96e78d62e..40cfa4b9211b2 100644
--- a/llvm/lib/Target/VE/VEInstrInfo.td
+++ b/llvm/lib/Target/VE/VEInstrInfo.td
@@ -2018,10 +2018,6 @@ def GETSTACKTOP : Pseudo<(outs I64:$dst), (ins),
"# GET STACK TOP",
[(set iPTR:$dst, (GetStackTop))]>;
-// MEMBARRIER
-let hasSideEffects = 1 in
-def MEMBARRIER : Pseudo<(outs), (ins), "# MEMBARRIER", [(membarrier)] >;
-
//===----------------------------------------------------------------------===//
// Other patterns
//===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/X86/X86InstrCompiler.td b/llvm/lib/Target/X86/X86InstrCompiler.td
index 44a77a41d9fdd..823784ce7e989 100644
--- a/llvm/lib/Target/X86/X86InstrCompiler.td
+++ b/llvm/lib/Target/X86/X86InstrCompiler.td
@@ -681,11 +681,6 @@ def OR32mi8Locked : Ii8<0x83, MRM1m, (outs), (ins i32mem:$dst, i32i8imm:$zero),
Requires<[Not64BitMode]>, OpSize32, LOCK,
Sched<[WriteALURMW]>;
-let hasSideEffects = 1, isMeta = 1 in
-def Int_MemBarrier : I<0, Pseudo, (outs), (ins),
- "#MEMBARRIER",
- [(membarrier)]>, Sched<[WriteLoad]>;
-
// RegOpc corresponds to the mr version of the instruction
// ImmOpc corresponds to the mi version of the instruction
// ImmOpc8 corresponds to the mi8 version of the instruction
diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp
index 9fb97df01b081..4d5e3789336ed 100644
--- a/llvm/lib/Target/X86/X86MCInstLower.cpp
+++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -2506,11 +2506,6 @@ void X86AsmPrinter::emitInstruction(const MachineInstr *MI) {
case TargetOpcode::DBG_VALUE:
llvm_unreachable("Should be handled target independently");
- // Emit nothing here but a comment if we can.
- case X86::Int_MemBarrier:
- OutStreamer->emitRawComment("MEMBARRIER");
- return;
-
case X86::EH_RETURN:
case X86::EH_RETURN64: {
// Lower these as normal, but add some comments.
diff --git a/llvm/lib/Target/XCore/XCoreInstrInfo.td b/llvm/lib/Target/XCore/XCoreInstrInfo.td
index e596966067405..f2b42d42b5a60 100644
--- a/llvm/lib/Target/XCore/XCoreInstrInfo.td
+++ b/llvm/lib/Target/XCore/XCoreInstrInfo.td
@@ -358,10 +358,6 @@ let usesCustomInserter = 1 in {
(select GRRegs:$cond, GRRegs:$T, GRRegs:$F))]>;
}
-let hasSideEffects = 1, isMeta = 1 in
-def Int_MemBarrier : PseudoInstXCore<(outs), (ins), "#MEMBARRIER",
- [(membarrier)]>;
-
//===----------------------------------------------------------------------===//
// Instructions
//===----------------------------------------------------------------------===//
diff --git a/llvm/test/CodeGen/X86/atomic-idempotent.ll b/llvm/test/CodeGen/X86/atomic-idempotent.ll
index 714f2912086cc..ef9ed79363668 100644
--- a/llvm/test/CodeGen/X86/atomic-idempotent.ll
+++ b/llvm/test/CodeGen/X86/atomic-idempotent.ll
@@ -359,6 +359,8 @@ define void @or32_nouse_monotonic(ptr %p) {
; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: nop
+; X86-ATOM-NEXT: nop
+; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: retl
atomicrmw or ptr %p, i32 0 monotonic
ret void
@@ -385,6 +387,8 @@ define void @or32_nouse_acquire(ptr %p) {
; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: nop
+; X86-ATOM-NEXT: nop
+; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: retl
atomicrmw or ptr %p, i32 0 acquire
ret void
@@ -410,6 +414,8 @@ define void @or32_nouse_release(ptr %p) {
; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: nop
+; X86-ATOM-NEXT: nop
+; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: retl
atomicrmw or ptr %p, i32 0 release
ret void
@@ -435,6 +441,8 @@ define void @or32_nouse_acq_rel(ptr %p) {
; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: nop
+; X86-ATOM-NEXT: nop
+; X86-ATOM-NEXT: nop
; X86-ATOM-NEXT: retl
atomicrmw or ptr %p, i32 0 acq_rel
ret void
diff --git a/llvm/unittests/MIR/MachineMetadata.cpp b/llvm/unittests/MIR/MachineMetadata.cpp
index f192b942190cf..f50e9b562942b 100644
--- a/llvm/unittests/MIR/MachineMetadata.cpp
+++ b/llvm/unittests/MIR/MachineMetadata.cpp
@@ -334,7 +334,7 @@ body: |
LIFETIME_END 0
PSEUDO_PROBE 6699318081062747564, 1, 0, 0
$xmm0 = ARITH_FENCE $xmm0
- Int_MemBarrier
+ MEMBARRIER
...
)MIR";
More information about the llvm-commits
mailing list