[SPIR-V] Fix OpDecorate emission after vreg def.

[Keenuts]

In SPIR-V, OpDecorate instructions are allowed to forward-declare a virtual register. But while we are at the MIR level, we must comply with stricter rules, meaning OpDecorate should be emited after, not before the reg definition.
(In some cases, we defined those just before, switching to just after).

Related to #110652

[llvmbot]

@llvm/pr-subscribers-backend-spir-v

Author: Nathan Gauër (Keenuts)

Changes

In SPIR-V, OpDecorate instructions are allowed to forward-declare a virtual register. But while we are at the MIR level, we must comply with stricter rules, meaning OpDecorate should be emited after, not before the reg definition.
(In some cases, we defined those just before, switching to just after).

Related to #110652

---

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

3 Files Affected:

• (modified) llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp (+1-1)
• (modified) llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp (+1-1)
• (added) llvm/test/CodeGen/SPIRV/decoration-order.ll (+15)

diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 526305d7ed28ab..892912a5680113 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -990,13 +990,13 @@ bool SPIRVInstructionSelector::selectMemOperation(Register ResVReg,
     Register VarReg = MRI->createGenericVirtualRegister(LLT::scalar(64));
     GR.add(GV, GR.CurMF, VarReg);
 
-    buildOpDecorate(VarReg, I, TII, SPIRV::Decoration::Constant, {});
     BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(SPIRV::OpVariable))
         .addDef(VarReg)
         .addUse(GR.getSPIRVTypeID(VarTy))
         .addImm(SPIRV::StorageClass::UniformConstant)
         .addUse(Const)
         .constrainAllUses(TII, TRI, RBI);
+    buildOpDecorate(VarReg, I, TII, SPIRV::Decoration::Constant, {});
     SPIRVType *SourceTy = GR.getOrCreateSPIRVPointerType(
         ValTy, I, TII, SPIRV::StorageClass::UniformConstant);
     SrcReg = MRI->createGenericVirtualRegister(LLT::scalar(64));
diff --git a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
index cc34cf877dea97..790d86f191fd86 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
@@ -829,7 +829,7 @@ static void insertSpirvDecorations(MachineFunction &MF, MachineIRBuilder MIB) {
     for (MachineInstr &MI : MBB) {
       if (!isSpvIntrinsic(MI, Intrinsic::spv_assign_decoration))
         continue;
-      MIB.setInsertPt(*MI.getParent(), MI);
+      MIB.setInsertPt(*MI.getParent(), MI.getNextNode());
       buildOpSpirvDecorations(MI.getOperand(1).getReg(), MIB,
                               MI.getOperand(2).getMetadata());
       ToErase.push_back(&MI);
diff --git a/llvm/test/CodeGen/SPIRV/decoration-order.ll b/llvm/test/CodeGen/SPIRV/decoration-order.ll
new file mode 100644
index 00000000000000..b6ffb6e5bd39cc
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/decoration-order.ll
@@ -0,0 +1,15 @@
+; RUN: %if spirv-tools %{ llc -O0 -verify-machineinstrs -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+; This test checks the OpDecorate MIR is generated after the associated
+; vreg definition in the case of an array size declared through this lowering.
+
+define spir_func i32 @foo(ptr addrspace(4) %Buf, ptr addrspace(4) %Item) {
+entry:
+  %var = alloca i64
+  br label %block
+
+block:
+  call void @llvm.memset.p0.i64(ptr align 8 %var, i8 0, i64 24, i1 false)
+  ret i32 0
+}
+
+declare void @llvm.memset.p0.i64(ptr nocapture writeonly, i8, i64, i1 immarg)

[Keenuts]

Thanks for the review! Rebased on main. Merging once green.

[Keenuts]

There are 2 broken tests, also broken on main:

• CodeGen/SPIRV/transcoding/RelationalOperators.ll
• CodeGen/SPIRV/uitofp-with-bool.ll
  Those are unrelated to this PR.

[VyacheslavLevytskyy]

There are 2 broken tests, also broken on main:

• CodeGen/SPIRV/transcoding/RelationalOperators.ll
• CodeGen/SPIRV/uitofp-with-bool.ll
  Those are unrelated to this PR.

@Keenuts I'm checking this now.

[Keenuts]

There are 2 broken tests, also broken on main:

• CodeGen/SPIRV/transcoding/RelationalOperators.ll
• CodeGen/SPIRV/uitofp-with-bool.ll
  Those are unrelated to this PR.

@Keenuts I'm checking this now.

Looks like this was fixed by #114630 (comment)

[VyacheslavLevytskyy]

Thank you @Keenuts