[CodeGen] Add SSID & Atomic Ordering to IntrinsicInfo

[Pierre-vh]

getTgtMemIntrinsic should be able to propagate such information to the MMO

[Pierre-vh]

• [CodeGen] Add SSID & Atomic Ordering to IntrinsicInfo #140896 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-llvm-globalisel

Author: Pierre van Houtryve (Pierre-vh)

Changes

getTgtMemIntrinsic should be able to propagate such information to the MMO

---

Full diff: https://github.com/llvm/llvm-project/pull/140896.diff

3 Files Affected:

• (modified) llvm/include/llvm/CodeGen/TargetLowering.h (+3)
• (modified) llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp (+3-2)
• (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+10-3)

diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 03099e9ad44dc..ef44c117acd1a 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -1214,6 +1214,9 @@ class TargetLoweringBase {
   struct IntrinsicInfo {
     unsigned     opc = 0;          // target opcode
     EVT          memVT;            // memory VT
+    SyncScope::ID ssid = SyncScope::System;
+    AtomicOrdering order = AtomicOrdering::NotAtomic;
+    AtomicOrdering failureOrder = AtomicOrdering::NotAtomic;
 
     // value representing memory location
     PointerUnion<const Value *, const PseudoSourceValue *> ptrVal;
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 8ab2533afc15f..051d220255876 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2847,8 +2847,9 @@ bool IRTranslator::translateCall(const User &U, MachineIRBuilder &MIRBuilder) {
       MPI = MachinePointerInfo(Info.ptrVal, Info.offset);
     else if (Info.fallbackAddressSpace)
       MPI = MachinePointerInfo(*Info.fallbackAddressSpace);
-    MIB.addMemOperand(
-        MF->getMachineMemOperand(MPI, Info.flags, MemTy, Alignment, CI.getAAMetadata()));
+    MIB.addMemOperand(MF->getMachineMemOperand(
+        MPI, Info.flags, MemTy, Alignment, CI.getAAMetadata(),
+        /*Ranges=*/nullptr, Info.ssid, Info.order, Info.failureOrder));
   }
 
   if (CI.isConvergent()) {
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index e32fcfe6148df..434484b671bf2 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -5305,9 +5305,16 @@ void SelectionDAGBuilder::visitTargetIntrinsic(const CallInst &I,
       MPI = MachinePointerInfo(Info.ptrVal, Info.offset);
     else if (Info.fallbackAddressSpace)
       MPI = MachinePointerInfo(*Info.fallbackAddressSpace);
-    Result = DAG.getMemIntrinsicNode(
-        Info.opc, getCurSDLoc(), VTs, Ops, Info.memVT, MPI, Info.align,
-        Info.flags, LocationSize::precise(Info.size), I.getAAMetadata());
+    EVT MemVT = Info.memVT;
+    LocationSize Size = LocationSize::precise(Info.size);
+    if (Size.hasValue() && !Size.getValue())
+      Size = LocationSize::precise(MemVT.getStoreSize());
+    Align Alignment = Info.align.value_or(DAG.getEVTAlign(MemVT));
+    MachineMemOperand *MMO = DAG.getMachineFunction().getMachineMemOperand(
+        MPI, Info.flags, Size, Alignment, I.getAAMetadata(), /*Ranges=*/nullptr,
+        Info.ssid, Info.order, Info.failureOrder);
+    Result =
+        DAG.getMemIntrinsicNode(Info.opc, getCurSDLoc(), VTs, Ops, MemVT, MMO);
   } else if (!HasChain) {
     Result = DAG.getNode(ISD::INTRINSIC_WO_CHAIN, getCurSDLoc(), VTs, Ops);
   } else if (!I.getType()->isVoidTy()) {

[arsenm]

MaybeAlign should not exist. Particularly in this case, where the MaybeAlign is initialized to a concrete Align(1)

[Pierre-vh]

Some targets use .reset() to make TLI infer the size based on memVT
AMDGPU, NVPTX and AArch64 all do it

[arsenm]

Which just shouldn't be done

[arsenm]

I don't understand why you need to do anything related to the size here

[Pierre-vh]

This is borrowed from SelectionDAG::getMemIntrinsicNode. Without it, I get size mismatches because there is an expectation that the value is initialized if it's zero.

[arsenm]

I think this whole API needs an overhaul, it's a collection of hacking around making changes to existing implementations. It would be better if LocationSize was directly constructed from MemVT in the first place

[arsenm]

In a follow up we should start reporting atomicity for the image atomic intrinsics

[arsenm]

I think this whole API needs an overhaul, it's a collection of hacking around making changes to existing implementations. It would be better if LocationSize was directly constructed from MemVT in the first place