Revert "[DXIL] Add GroupMemoryBarrierWithGroupSync intrinsic"

[adam-yang]

Reverts #111884

[llvmbot]

@llvm/pr-subscribers-tablegen
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-backend-directx

Author: Adam Yang (adam-yang)

Changes

Reverts llvm/llvm-project#111884

---

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

5 Files Affected:

• (modified) llvm/include/llvm/IR/IntrinsicsDirectX.td (-2)
• (modified) llvm/lib/Target/DirectX/DXIL.td (-54)
• (modified) llvm/lib/Target/DirectX/DXILOpLowering.cpp (+9-36)
• (removed) llvm/test/CodeGen/DirectX/group_memory_barrier_with_group_sync.ll (-8)
• (modified) llvm/utils/TableGen/DXILEmitter.cpp (+13-109)

diff --git a/llvm/include/llvm/IR/IntrinsicsDirectX.td b/llvm/include/llvm/IR/IntrinsicsDirectX.td
index dada426368995d..e30d37f69f781e 100644
--- a/llvm/include/llvm/IR/IntrinsicsDirectX.td
+++ b/llvm/include/llvm/IR/IntrinsicsDirectX.td
@@ -92,6 +92,4 @@ def int_dx_step : DefaultAttrsIntrinsic<[LLVMMatchType<0>], [llvm_anyfloat_ty, L
 def int_dx_splitdouble : DefaultAttrsIntrinsic<[llvm_anyint_ty, LLVMMatchType<0>], 
     [LLVMScalarOrSameVectorWidth<0, llvm_double_ty>], [IntrNoMem]>;
 def int_dx_radians : DefaultAttrsIntrinsic<[llvm_anyfloat_ty], [LLVMMatchType<0>], [IntrNoMem]>;
-
-def int_dx_group_memory_barrier_with_group_sync : DefaultAttrsIntrinsic<[], [], []>;
 }
diff --git a/llvm/lib/Target/DirectX/DXIL.td b/llvm/lib/Target/DirectX/DXIL.td
index 263ca50011aa7b..1e8dc63ffa257e 100644
--- a/llvm/lib/Target/DirectX/DXIL.td
+++ b/llvm/lib/Target/DirectX/DXIL.td
@@ -294,43 +294,6 @@ class Attributes<Version ver = DXIL1_0, list<DXILAttribute> attrs> {
   list<DXILAttribute> op_attrs = attrs;
 }
 
-class DXILConstant<int value_> {
-  int value = value_;
-}
-
-defset list<DXILConstant> BarrierModes = {
-  def BarrierMode_DeviceMemoryBarrier              : DXILConstant<2>;
-  def BarrierMode_DeviceMemoryBarrierWithGroupSync : DXILConstant<3>;
-  def BarrierMode_GroupMemoryBarrier               : DXILConstant<8>;
-  def BarrierMode_GroupMemoryBarrierWithGroupSync  : DXILConstant<9>;
-  def BarrierMode_AllMemoryBarrier                 : DXILConstant<10>;
-  def BarrierMode_AllMemoryBarrierWithGroupSync    : DXILConstant<11>;
-}
-
-// Intrinsic arg selection
-class Arg {
-  int index = -1;
-  DXILConstant value;
-  bit is_i8 = 0;
-  bit is_i32 = 0;
-}
-class ArgSelect<int index_> : Arg {
-  let index = index_;
-}
-class ArgI32<DXILConstant value_> : Arg {
-  let value = value_;
-  let is_i32 = 1;
-}
-class ArgI8<DXILConstant value_> : Arg {
-  let value = value_;
-  let is_i8 = 1;
-}
-
-class IntrinsicSelect<Intrinsic intrinsic_, list<Arg> args_> {
-  Intrinsic intrinsic = intrinsic_;
-  list<Arg> args = args_;
-}
-
 // Abstraction DXIL Operation
 class DXILOp<int opcode, DXILOpClass opclass> {
   // A short description of the operation
@@ -345,9 +308,6 @@ class DXILOp<int opcode, DXILOpClass opclass> {
   // LLVM Intrinsic DXIL Operation maps to
   Intrinsic LLVMIntrinsic = ?;
 
-  // Non-trivial LLVM Intrinsics DXIL Operation maps to
-  list<IntrinsicSelect> intrinsic_selects = [];
-
   // Result type of the op
   DXILOpParamType result;
 
@@ -869,17 +829,3 @@ def WaveGetLaneIndex : DXILOp<111, waveGetLaneIndex> {
   let stages = [Stages<DXIL1_0, [all_stages]>];
   let attributes = [Attributes<DXIL1_0, [ReadNone]>];
 }
-
-def Barrier : DXILOp<80, barrier> {
-  let Doc = "inserts a memory barrier in the shader";
-  let intrinsic_selects = [
-    IntrinsicSelect<
-        int_dx_group_memory_barrier_with_group_sync,
-        [ ArgI32<BarrierMode_GroupMemoryBarrierWithGroupSync> ]>,
-  ];
-
-  let arguments = [Int32Ty];
-  let result = VoidTy;
-  let stages = [Stages<DXIL1_0, [compute, library]>];
-  let attributes = [Attributes<DXIL1_0, []>];
-}
diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
index b5cf1654181c6c..8acc9c1efa08c0 100644
--- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
@@ -106,43 +106,17 @@ class OpLowerer {
     return false;
   }
 
-  struct ArgSelect {
-    enum class Type {
-      Index,
-      I8,
-      I32,
-    };
-    Type Type = Type::Index;
-    int Value = -1;
-  };
-
-  [[nodiscard]] bool replaceFunctionWithOp(Function &F, dxil::OpCode DXILOp,
-                                           ArrayRef<ArgSelect> ArgSelects) {
+  [[nodiscard]]
+  bool replaceFunctionWithOp(Function &F, dxil::OpCode DXILOp) {
     bool IsVectorArgExpansion = isVectorArgExpansion(F);
     return replaceFunction(F, [&](CallInst *CI) -> Error {
-      OpBuilder.getIRB().SetInsertPoint(CI);
       SmallVector<Value *> Args;
-      if (ArgSelects.size()) {
-        for (const ArgSelect &A : ArgSelects) {
-          switch (A.Type) {
-          case ArgSelect::Type::Index:
-            Args.push_back(CI->getArgOperand(A.Value));
-            break;
-          case ArgSelect::Type::I8:
-            Args.push_back(OpBuilder.getIRB().getInt8((uint8_t)A.Value));
-            break;
-          case ArgSelect::Type::I32:
-            Args.push_back(OpBuilder.getIRB().getInt32(A.Value));
-            break;
-          default:
-            llvm_unreachable("Invalid type of intrinsic arg select.");
-          }
-        }
-      } else if (IsVectorArgExpansion) {
-        Args = argVectorFlatten(CI, OpBuilder.getIRB());
-      } else {
+      OpBuilder.getIRB().SetInsertPoint(CI);
+      if (IsVectorArgExpansion) {
+        SmallVector<Value *> NewArgs = argVectorFlatten(CI, OpBuilder.getIRB());
+        Args.append(NewArgs.begin(), NewArgs.end());
+      } else
         Args.append(CI->arg_begin(), CI->arg_end());
-      }
 
       Expected<CallInst *> OpCall =
           OpBuilder.tryCreateOp(DXILOp, Args, CI->getName(), F.getReturnType());
@@ -609,10 +583,9 @@ class OpLowerer {
       switch (ID) {
       default:
         continue;
-#define DXIL_OP_INTRINSIC(OpCode, Intrin, ...)                                 \
+#define DXIL_OP_INTRINSIC(OpCode, Intrin)                                      \
   case Intrin:                                                                 \
-    HasErrors |=                                                               \
-        replaceFunctionWithOp(F, OpCode, ArrayRef<ArgSelect>{__VA_ARGS__});    \
+    HasErrors |= replaceFunctionWithOp(F, OpCode);                             \
     break;
 #include "DXILOperation.inc"
       case Intrinsic::dx_handle_fromBinding:
diff --git a/llvm/test/CodeGen/DirectX/group_memory_barrier_with_group_sync.ll b/llvm/test/CodeGen/DirectX/group_memory_barrier_with_group_sync.ll
deleted file mode 100644
index baf93d4e177f0f..00000000000000
--- a/llvm/test/CodeGen/DirectX/group_memory_barrier_with_group_sync.ll
+++ /dev/null
@@ -1,8 +0,0 @@
-; RUN: opt -S -dxil-op-lower -mtriple=dxil-pc-shadermodel6.3-library < %s | FileCheck %s
-
-define void @test_group_memory_barrier_with_group_sync() {
-entry:
-  ; CHECK: call void @dx.op.barrier(i32 80, i32 9)
-  call void @llvm.dx.group.memory.barrier.with.group.sync()
-  ret void
-}
\ No newline at end of file
diff --git a/llvm/utils/TableGen/DXILEmitter.cpp b/llvm/utils/TableGen/DXILEmitter.cpp
index 8bebe608eece47..e74fc00015b404 100644
--- a/llvm/utils/TableGen/DXILEmitter.cpp
+++ b/llvm/utils/TableGen/DXILEmitter.cpp
@@ -32,20 +32,6 @@ using namespace llvm::dxil;
 
 namespace {
 
-struct DXILArgSelect {
-  enum class Type {
-    Index,
-    I32,
-    I8,
-  };
-  Type Type = Type::Index;
-  int Value = -1;
-};
-struct DXILIntrinsicSelect {
-  StringRef Intrinsic;
-  SmallVector<DXILArgSelect, 4> Args;
-};
-
 struct DXILOperationDesc {
   std::string OpName; // name of DXIL operation
   int OpCode;         // ID of DXIL operation
@@ -56,7 +42,8 @@ struct DXILOperationDesc {
   SmallVector<const Record *> OverloadRecs;
   SmallVector<const Record *> StageRecs;
   SmallVector<const Record *> AttrRecs;
-  SmallVector<DXILIntrinsicSelect> IntrinsicSelects;
+  StringRef Intrinsic; // The llvm intrinsic map to OpName. Default is "" which
+                       // means no map exists
   SmallVector<StringRef, 4>
       ShaderStages; // shader stages to which this applies, empty for all.
   int OverloadParamIndex;             // Index of parameter with overload type.
@@ -84,21 +71,6 @@ static void ascendingSortByVersion(std::vector<const Record *> &Recs) {
   });
 }
 
-/// Take a `int_{intrinsic_name}` and return just the intrinsic_name part if
-/// available. Otherwise return the empty string.
-static StringRef GetIntrinsicName(const RecordVal *RV) {
-  if (RV && RV->getValue()) {
-    if (const DefInit *DI = dyn_cast<DefInit>(RV->getValue())) {
-      auto *IntrinsicDef = DI->getDef();
-      auto DefName = IntrinsicDef->getName();
-      assert(DefName.starts_with("int_") && "invalid intrinsic name");
-      // Remove the int_ from intrinsic name.
-      return DefName.substr(4);
-    }
-  }
-  return "";
-}
-
 /// Construct an object using the DXIL Operation records specified
 /// in DXIL.td. This serves as the single source of reference of
 /// the information extracted from the specified Record R, for
@@ -185,63 +157,14 @@ DXILOperationDesc::DXILOperationDesc(const Record *R) {
                            OpName);
   }
 
-  {
-    DXILIntrinsicSelect IntrSelect;
-    IntrSelect.Intrinsic = GetIntrinsicName(R->getValue("LLVMIntrinsic"));
-    if (IntrSelect.Intrinsic.size())
-      IntrinsicSelects.emplace_back(std::move(IntrSelect));
-  }
-
-  auto IntrinsicSelectRecords = R->getValueAsListOfDefs("intrinsic_selects");
-  if (IntrinsicSelectRecords.size()) {
-    if (IntrinsicSelects.size()) {
-      PrintFatalError(
-          R, Twine("LLVMIntrinsic and intrinsic_selects cannot be both "
-                   "defined for DXIL operation - ") +
-                 OpName);
-    } else {
-      for (const Record *R : IntrinsicSelectRecords) {
-        DXILIntrinsicSelect IntrSelect;
-        IntrSelect.Intrinsic = GetIntrinsicName(R->getValue("intrinsic"));
-        auto Args = R->getValueAsListOfDefs("args");
-        for (const Record *Arg : Args) {
-          bool IsI8 = Arg->getValueAsBit("is_i8");
-          bool IsI32 = Arg->getValueAsBit("is_i32");
-          int Index = Arg->getValueAsInt("index");
-          const Record *ValueRec = Arg->getValueAsOptionalDef("value");
-
-          DXILArgSelect ArgSelect;
-          if (IsI8) {
-            if (!ValueRec) {
-              PrintFatalError(R, Twine("'value' must be defined for i8 "
-                                       "ArgSelect for DXIL operation - ") +
-                                     OpName);
-            }
-            ArgSelect.Type = DXILArgSelect::Type::I8;
-            ArgSelect.Value = ValueRec->getValueAsInt("value");
-          } else if (IsI32) {
-            if (!ValueRec) {
-              PrintFatalError(R, Twine("'value' must be defined for i32 "
-                                       "ArgSelect for DXIL operation - ") +
-                                     OpName);
-            }
-            ArgSelect.Type = DXILArgSelect::Type::I32;
-            ArgSelect.Value = ValueRec->getValueAsInt("value");
-          } else {
-            if (Index < 0) {
-              PrintFatalError(
-                  R, Twine("Index in ArgSelect<index> must be equal to or "
-                           "greater than 0 for DXIL operation - ") +
-                         OpName);
-            }
-            ArgSelect.Type = DXILArgSelect::Type::Index;
-            ArgSelect.Value = Index;
-          }
-
-          IntrSelect.Args.emplace_back(std::move(ArgSelect));
-        }
-        IntrinsicSelects.emplace_back(std::move(IntrSelect));
-      }
+  const RecordVal *RV = R->getValue("LLVMIntrinsic");
+  if (RV && RV->getValue()) {
+    if (const DefInit *DI = dyn_cast<DefInit>(RV->getValue())) {
+      auto *IntrinsicDef = DI->getDef();
+      auto DefName = IntrinsicDef->getName();
+      assert(DefName.starts_with("int_") && "invalid intrinsic name");
+      // Remove the int_ from intrinsic name.
+      Intrinsic = DefName.substr(4);
     }
   }
 }
@@ -454,29 +377,10 @@ static void emitDXILIntrinsicMap(ArrayRef<DXILOperationDesc> Ops,
   OS << "#ifdef DXIL_OP_INTRINSIC\n";
   OS << "\n";
   for (const auto &Op : Ops) {
-    if (Op.IntrinsicSelects.empty()) {
+    if (Op.Intrinsic.empty())
       continue;
-    }
-    for (const DXILIntrinsicSelect &MappedIntr : Op.IntrinsicSelects) {
-      OS << "DXIL_OP_INTRINSIC(dxil::OpCode::" << Op.OpName
-         << ", Intrinsic::" << MappedIntr.Intrinsic;
-      for (const DXILArgSelect &ArgSelect : MappedIntr.Args) {
-        OS << ", (ArgSelect { ";
-        switch (ArgSelect.Type) {
-        case DXILArgSelect::Type::Index:
-          OS << "ArgSelect::Type::Index, ";
-          break;
-        case DXILArgSelect::Type::I8:
-          OS << "ArgSelect::Type::I8, ";
-          break;
-        case DXILArgSelect::Type::I32:
-          OS << "ArgSelect::Type::I32, ";
-          break;
-        }
-        OS << ArgSelect.Value << "})";
-      }
-      OS << ")\n";
-    }
+    OS << "DXIL_OP_INTRINSIC(dxil::OpCode::" << Op.OpName
+       << ", Intrinsic::" << Op.Intrinsic << ")\n";
   }
   OS << "\n";
   OS << "#undef DXIL_OP_INTRINSIC\n";