[AMDGPU] Handle non-register operands for S_SUB/ADD_U64_PSEUDO

[Pierre-vh]

This pseudo uses SSrc_b64 so it allows both an immediate or a register, but the lowering crashed on immediate operands.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Pierre van Houtryve (Pierre-vh)

Changes

This pseudo uses SSrc_b64 so it allows both an immediate or a register, but the lowering crashed on register operands.

---

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

1 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/SIISelLowering.cpp (+2-2)

diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index 5ccf21f76015de..045095684e845c 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -4858,8 +4858,8 @@ MachineBasicBlock *SITargetLowering::EmitInstrWithCustomInserter(
     if (Subtarget->hasScalarAddSub64()) {
       unsigned Opc = IsAdd ? AMDGPU::S_ADD_U64 : AMDGPU::S_SUB_U64;
       BuildMI(*BB, MI, DL, TII->get(Opc), Dest.getReg())
-          .addReg(Src0.getReg())
-          .addReg(Src1.getReg());
+          .add(Src0)
+          .add(Src1);
     } else {
       const SIRegisterInfo *TRI = ST.getRegisterInfo();
       const TargetRegisterClass *BoolRC = TRI->getBoolRC();

[arsenm]

Missing test

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 23de3862dce582ce91c1aa914467d982cb1a73b4 ef6356c5b6abf5a01014074e67e8dc52360aa957 -- llvm/lib/Target/AMDGPU/SIISelLowering.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index e91136f66e..5a00d63b9b 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -4857,9 +4857,7 @@ MachineBasicBlock *SITargetLowering::EmitInstrWithCustomInserter(
     bool IsAdd = (MI.getOpcode() == AMDGPU::S_ADD_U64_PSEUDO);
     if (Subtarget->hasScalarAddSub64()) {
       unsigned Opc = IsAdd ? AMDGPU::S_ADD_U64 : AMDGPU::S_SUB_U64;
-      BuildMI(*BB, MI, DL, TII->get(Opc), Dest.getReg())
-        .add(Src0)
-        .add(Src1);
+      BuildMI(*BB, MI, DL, TII->get(Opc), Dest.getReg()).add(Src0).add(Src1);
     } else {
       const SIRegisterInfo *TRI = ST.getRegisterInfo();
       const TargetRegisterClass *BoolRC = TRI->getBoolRC();

[Pierre-vh]

Missing test

How can I test ISel pseudo expansion? I didn't find the tests for it

[Pierre-vh]

Missing test

How can I test ISel pseudo expansion? I didn't find the tests for it

This somewhat depends on #79553, we currently don't emit those pseudos so it can't be tested. Either that patch needs to add a test for it, or this whole patch should be merged with it

[jayfoad]

but the lowering crashed on register operands

Crashed on non-register operands?

[arsenm]

Missing test

How can I test ISel pseudo expansion? I didn't find the tests for it

Ideally there would be a test that generated the failing pseudo in the first place, but you can also just run finalize-isel on MIR

[Pierre-vh]

but the lowering crashed on register operands

Crashed on non-register operands?

Oops, I meant immediate

[arsenm]

A test that really selected this way would be better