[SPIR-V] Fix legalize info for G_BITREVERSE

[VyacheslavLevytskyy]

This PR fixes legalize info for G_BITREVERSE.

[llvmbot]

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

Author: Vyacheslav Levytskyy (VyacheslavLevytskyy)

Changes

This PR fixes legalize info for G_BITREVERSE.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp (+1-1)
• (modified) llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp (+5)

diff --git a/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp b/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
index 42d36fd30ed6f..57fbf3b3f8f12 100644
--- a/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
@@ -182,7 +182,7 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) {
 
   getActionDefinitionsBuilder({G_LOAD, G_STORE}).legalIf(typeInSet(1, allPtrs));
 
-  getActionDefinitionsBuilder(G_BITREVERSE).legalFor(allFloatScalarsAndVectors);
+  getActionDefinitionsBuilder(G_BITREVERSE).legalFor(allIntScalarsAndVectors);
 
   getActionDefinitionsBuilder(G_FMA).legalFor(allFloatScalarsAndVectors);
 
diff --git a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
index 624899600693a..3d536085b78aa 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
@@ -472,6 +472,11 @@ generateAssignInstrs(MachineFunction &MF, SPIRVGlobalRegistry *GR,
         insertAssignInstr(Reg, Ty, nullptr, GR, MIB, MRI);
       } else if (MIOp == TargetOpcode::G_GLOBAL_VALUE) {
         propagateSPIRVType(&MI, GR, MRI, MIB);
+      } else if (MIOp == TargetOpcode::G_BITREVERSE) {
+        Register Reg = MI.getOperand(0).getReg();
+        LLT RegType = MRI.getType(Reg);
+        if (RegType.getSizeInBits() < 32)
+          MRI.setType(Reg, LLT::scalar(32));
       }
 
       if (MII == Begin)

[tschuett]

Drive by nit: Why were there no test changes, when you touched the legalizer?

[farzonl]

I think HLSL should have a reversebits test that might be affected by this change, can you confirm?

[tschuett]

It originated from https://reviews.llvm.org/D116464, which is a huge patch with no tests.

[VyacheslavLevytskyy]

Drive by nit: Why were there no test changes, when you touched the legalizer?
It originated from https://reviews.llvm.org/D116464, which is a huge patch with no tests.

@tschuett Yes, it looks like legalizer wasn't covered by tests properly. It's one of our priorities now to improve test cases.

[VyacheslavLevytskyy]

I think HLSL should have a reversebits test that might be affected by this change, can you confirm?

@farzonl I don't expect HLSL's reversebits test to be affected, as it uses i16 and i32 for bitreverse. This PR is a start of fixing the issue with llvm.bitreverse applied to other than 8,16,32,64 integer sizes.

[VyacheslavLevytskyy]

Failed CI builds are not related to the PR content.