[clang] [llvm] [HLSL] Adding Flatten and Branch if attributes (PR #116331)

[Chris B via llvm-commits]

================
@@ -2694,19 +2694,49 @@ bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
     }
     return MIB.constrainAllUses(TII, TRI, RBI);
   }
-  case Intrinsic::spv_loop_merge:
-  case Intrinsic::spv_selection_merge: {
-    const auto Opcode = IID == Intrinsic::spv_selection_merge
-                            ? SPIRV::OpSelectionMerge
-                            : SPIRV::OpLoopMerge;
-    auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(Opcode));
+  case Intrinsic::spv_loop_merge: {
+    auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpLoopMerge));
     for (unsigned i = 1; i < I.getNumExplicitOperands(); ++i) {
       assert(I.getOperand(i).isMBB());
       MIB.addMBB(I.getOperand(i).getMBB());
     }
     MIB.addImm(SPIRV::SelectionControl::None);
     return MIB.constrainAllUses(TII, TRI, RBI);
   }
+  case Intrinsic::spv_selection_merge: {
+
+    auto SelectionControl = SPIRV::SelectionControl::None;
+    auto LastOp = I.getOperand(I.getNumExplicitOperands() - 1);
----------------
llvm-beanz wrote:

Stepping back. First off, we don't actually have a test showing the IR post-SPIRVStructurizer, so that part of your change isn't unit tested. The transformation performed in that pass actually might be the root of my concern here.

It looks like you're translating the metadata from the terminator instruction into an argument to a SPIR-V intrinsic, then lowering that intrinsic. That isn't inherently bad, but it makes it a really strange decision to keep the metadata in its language-specific named metadata form as an argument to an intrinsic.

It seems like we should probably translate the metadata into an immediate value passed to the intrinsic rather than keeping it as metadata.

https://github.com/llvm/llvm-project/pull/116331