[llvm] [SPIRV] Add support for SPV_KHR_bit_instructions (PR #66215)
Paulo Matos via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 22 05:43:27 PDT 2023
https://github.com/pmatos updated https://github.com/llvm/llvm-project/pull/66215
>From 86375a2cdaeccac8201a7c1cf81963ade8cef14c Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Wed, 13 Sep 2023 16:38:35 +0200
Subject: [PATCH 1/7] [SPIRV] Add support for SPV_KHR_bit_instructions
Draft version of patch to add support for SPV_KHR_bit_instructions.
Created revision so we can discuss how to proceed here.
SPV_KHR_bit_instructions (http://htmlpreview.github.io/?https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_bit_instructions.html),
adds support for instructions that we already support through
the Shader capability but without requiring the shader cap.
However we currently have no way to disable the shader cap afaiu.
On the other hand, SPV_KHR_bit_instructions was one of the
extensions @mpaszkowski listed that we need to implement.
So what does it mean at the moment the need to implement this?
Is it just the ability to issue the extension and capabilities
instructions without issuing the Shader capability? If so,
then I will need to add here a way to disable the Shader
capability. What do you think?
Differential Revision: https://reviews.llvm.org/D156661
---
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | 19 +++++++++++++++++++
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h | 4 ++++
llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp | 1 +
.../lib/Target/SPIRV/SPIRVSymbolicOperands.td | 1 +
.../SPIRV/transcoding/OpBitReverse_i32.ll | 3 +++
5 files changed, 28 insertions(+)
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index 3754f57ef3ac703..ec713c547b13299 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -15,6 +15,8 @@
//===----------------------------------------------------------------------===//
#include "SPIRVModuleAnalysis.h"
+#include "MCTargetDesc/SPIRVBaseInfo.h"
+#include "MCTargetDesc/SPIRVMCTargetDesc.h"
#include "SPIRV.h"
#include "SPIRVSubtarget.h"
#include "SPIRVTargetMachine.h"
@@ -523,6 +525,12 @@ void SPIRV::RequirementHandler::addAvailableCaps(const CapabilityList &ToAdd) {
SPIRV::OperandCategory::CapabilityOperand, Cap));
}
+void SPIRV::RequirementHandler::removeCapabilityIf(const Capability::Capability ToRemove,
+ const Capability::Capability IfPresent) {
+ if (AvailableCaps.contains(IfPresent))
+ AvailableCaps.erase(ToRemove);
+}
+
namespace llvm {
namespace SPIRV {
void RequirementHandler::initAvailableCapabilities(const SPIRVSubtarget &ST) {
@@ -734,6 +742,11 @@ void addInstrRequirements(const MachineInstr &MI,
break;
}
case SPIRV::OpBitReverse:
+ case SPIRV::OpBitFieldInsert:
+ case SPIRV::OpBitFieldSExtract:
+ case SPIRV::OpBitFieldUExtract:
+ Reqs.addExtension(SPIRV::Extension::SPV_KHR_bit_instructions);
+ break;
case SPIRV::OpTypeRuntimeArray:
Reqs.addCapability(SPIRV::Capability::Shader);
break;
@@ -887,6 +900,12 @@ void addInstrRequirements(const MachineInstr &MI,
default:
break;
}
+
+ // If we require capability Shader, then we can remove the requirement for
+ // the BitInstructions capability, since Shader is a superset capability
+ // of BitInstructions.
+ Reqs.removeCapabilityIf(SPIRV::Capability::Shader,
+ SPIRV::Capability::BitInstructions);
}
static void collectReqs(const Module &M, SPIRV::ModuleAnalysisInfo &MAI,
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
index b57a5f4c28278ac..9eea9ac5bda5b08 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
@@ -113,6 +113,10 @@ struct RequirementHandler {
bool isCapabilityAvailable(Capability::Capability Cap) const {
return AvailableCaps.contains(Cap);
}
+
+ // Remove capability ToRemove, but only if IfPresent is present.
+ void removeCapabilityIf(const Capability::Capability ToRemove,
+ const Capability::Capability IfPresent);
};
using InstrList = SmallVector<MachineInstr *>;
diff --git a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
index 1bad1d8109623b6..fdf103941f4b95e 100644
--- a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
@@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//
#include "SPIRVSubtarget.h"
+#include "MCTargetDesc/SPIRVBaseInfo.h"
#include "SPIRV.h"
#include "SPIRVGlobalRegistry.h"
#include "SPIRVLegalizerInfo.h"
diff --git a/llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td b/llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td
index c27d2826c0159d1..ab06f5308700bc5 100644
--- a/llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td
+++ b/llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td
@@ -450,6 +450,7 @@ defm PhysicalStorageBufferAddressesEXT : CapabilityOperand<5347, 0, 0, [], [Shad
defm CooperativeMatrixNV : CapabilityOperand<5357, 0, 0, [], [Shader]>;
defm ArbitraryPrecisionIntegersINTEL : CapabilityOperand<5844, 0, 0, [SPV_INTEL_arbitrary_precision_integers], [Int8, Int16]>;
defm OptNoneINTEL : CapabilityOperand<6094, 0, 0, [SPV_INTEL_optnone], []>;
+defm BitInstructions : CapabilityOperand<6025, 0, 0, [SPV_KHR_bit_instructions], []>;
//===----------------------------------------------------------------------===//
// Multiclass used to define SourceLanguage enum values and at the same time
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpBitReverse_i32.ll b/llvm/test/CodeGen/SPIRV/transcoding/OpBitReverse_i32.ll
index f396b5a01ae91c1..4f95c4770780432 100644
--- a/llvm/test/CodeGen/SPIRV/transcoding/OpBitReverse_i32.ll
+++ b/llvm/test/CodeGen/SPIRV/transcoding/OpBitReverse_i32.ll
@@ -1,5 +1,8 @@
; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
+; CHECK-SPIRV: OpCapability BitInstructions
+; CHECK-SPIRV-NEXT: OpExtension "SPV_KHR_bit_instructions"
+
; CHECK-SPIRV: %[[#int:]] = OpTypeInt 32
; CHECK-SPIRV: OpBitReverse %[[#int]]
>From 7cfad0278cac91fcec1a1eada5382f54d07396c4 Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Fri, 15 Sep 2023 16:55:04 +0200
Subject: [PATCH 2/7] Add test and enable SPV_KHR_bit_instructions
---
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | 7 +++++-
llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp | 6 ++++-
.../extensions/SPV_KHR_bit_instructions.ll | 24 +++++++++++++++++++
.../SPIRV/transcoding/OpBitReverse_i32.ll | 3 ---
4 files changed, 35 insertions(+), 5 deletions(-)
create mode 100644 llvm/test/CodeGen/SPIRV/extensions/SPV_KHR_bit_instructions.ll
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index ec713c547b13299..a4160f1a0385b59 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -506,7 +506,7 @@ void SPIRV::RequirementHandler::checkSatisfiable(
for (auto Ext : AllExtensions) {
if (ST.canUseExtension(Ext))
continue;
- LLVM_DEBUG(dbgs() << "Extension not suported: "
+ LLVM_DEBUG(dbgs() << "Extension not supported: "
<< getSymbolicOperandMnemonic(
OperandCategory::ExtensionOperand, Ext)
<< "\n");
@@ -745,7 +745,12 @@ void addInstrRequirements(const MachineInstr &MI,
case SPIRV::OpBitFieldInsert:
case SPIRV::OpBitFieldSExtract:
case SPIRV::OpBitFieldUExtract:
+ if (!ST.canUseExtension(SPIRV::Extension::SPV_KHR_bit_instructions)) {
+ Reqs.addCapability(SPIRV::Capability::Shader);
+ break;
+ }
Reqs.addExtension(SPIRV::Extension::SPV_KHR_bit_instructions);
+ Reqs.addCapability(SPIRV::Capability::BitInstructions);
break;
case SPIRV::OpTypeRuntimeArray:
Reqs.addCapability(SPIRV::Capability::Shader);
diff --git a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
index fdf103941f4b95e..1584ed477fa4ff2 100644
--- a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
@@ -41,7 +41,11 @@ cl::list<SPIRV::Extension::Extension> Extensions(
clEnumValN(SPIRV::Extension::SPV_KHR_no_integer_wrap_decoration,
"SPV_KHR_no_integer_wrap_decoration",
"Adds decorations to indicate that a given instruction does "
- "not cause integer wrapping")));
+ "not cause integer wrapping"),
+ clEnumValN(SPIRV::Extension::SPV_KHR_bit_instructions,
+ "SPV_KHR_bit_instructions",
+ "This enables bit instructions to be used by SPIR-V modules "
+ "without the requiring the Shader capability")));
// Compare version numbers, but allow 0 to mean unspecified.
static bool isAtLeastVer(uint32_t Target, uint32_t VerToCompareTo) {
diff --git a/llvm/test/CodeGen/SPIRV/extensions/SPV_KHR_bit_instructions.ll b/llvm/test/CodeGen/SPIRV/extensions/SPV_KHR_bit_instructions.ll
new file mode 100644
index 000000000000000..95395d5efb55d8b
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/extensions/SPV_KHR_bit_instructions.ll
@@ -0,0 +1,24 @@
+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s --spirv-extensions=SPV_KHR_bit_instructions -o - | FileCheck %s --check-prefix=CHECK-EXTENSION
+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-NO-EXTENSION
+
+; CHECK-EXTENSION: OpCapability BitInstructions
+; CHECK-EXTENSION-NEXT: OpExtension "SPV_KHR_bit_instructions"
+; CHECK-EXTENSION-NOT: OpCabilitity Shader
+; CHECK-NO-EXTENSION: OpCapability Shader
+; CHECK-NO-EXTENSION-NOT: OpCabilitity BitInstructions
+; CHECK-NO-EXTENSION-NOT: OpExtension "SPV_KHR_bit_instructions"
+
+
+; CHECK-EXTENSION: %[[#int:]] = OpTypeInt 32
+; CHECK-EXTENSION: OpBitReverse %[[#int]]
+; CHECK-NO-EXTENSION: %[[#int:]] = OpTypeInt 32
+; CHECK-NO-EXTENSION: OpBitReverse %[[#int]]
+
+define spir_kernel void @testBitRev(i32 %a, i32 %b, i32 %c, i32 addrspace(1)* nocapture %res) local_unnamed_addr {
+entry:
+ %call = tail call i32 @llvm.bitreverse.i32(i32 %b)
+ store i32 %call, i32 addrspace(1)* %res, align 4
+ ret void
+}
+
+declare i32 @llvm.bitreverse.i32(i32)
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpBitReverse_i32.ll b/llvm/test/CodeGen/SPIRV/transcoding/OpBitReverse_i32.ll
index 4f95c4770780432..f396b5a01ae91c1 100644
--- a/llvm/test/CodeGen/SPIRV/transcoding/OpBitReverse_i32.ll
+++ b/llvm/test/CodeGen/SPIRV/transcoding/OpBitReverse_i32.ll
@@ -1,8 +1,5 @@
; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
-; CHECK-SPIRV: OpCapability BitInstructions
-; CHECK-SPIRV-NEXT: OpExtension "SPV_KHR_bit_instructions"
-
; CHECK-SPIRV: %[[#int:]] = OpTypeInt 32
; CHECK-SPIRV: OpBitReverse %[[#int]]
>From 980252c159d722860f5914461ada5099b2f4a10e Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Fri, 22 Sep 2023 11:38:48 +0200
Subject: [PATCH 3/7] Remove BitInstructions if Shader is present
---
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index a4160f1a0385b59..03891f4c815e05c 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -909,8 +909,7 @@ void addInstrRequirements(const MachineInstr &MI,
// If we require capability Shader, then we can remove the requirement for
// the BitInstructions capability, since Shader is a superset capability
// of BitInstructions.
- Reqs.removeCapabilityIf(SPIRV::Capability::Shader,
- SPIRV::Capability::BitInstructions);
+ Reqs.removeCapabilityIf(SPIRV::Capability::BitInstructions, SPIRV::Capability::Shader);
}
static void collectReqs(const Module &M, SPIRV::ModuleAnalysisInfo &MAI,
>From a5db350d1c1db8fe3475612d2118b37d5f25da22 Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Fri, 22 Sep 2023 12:22:50 +0200
Subject: [PATCH 4/7] clang-format patch
---
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index 03891f4c815e05c..fb38ac3dd370524 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -909,7 +909,8 @@ void addInstrRequirements(const MachineInstr &MI,
// If we require capability Shader, then we can remove the requirement for
// the BitInstructions capability, since Shader is a superset capability
// of BitInstructions.
- Reqs.removeCapabilityIf(SPIRV::Capability::BitInstructions, SPIRV::Capability::Shader);
+ Reqs.removeCapabilityIf(SPIRV::Capability::BitInstructions,
+ SPIRV::Capability::Shader);
}
static void collectReqs(const Module &M, SPIRV::ModuleAnalysisInfo &MAI,
>From c69065e34bc6df06042bdc8da4d2396dd42e7081 Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Fri, 22 Sep 2023 12:24:11 +0200
Subject: [PATCH 5/7] clang-format missing hunk
---
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index fb38ac3dd370524..72162204afc823b 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -525,8 +525,9 @@ void SPIRV::RequirementHandler::addAvailableCaps(const CapabilityList &ToAdd) {
SPIRV::OperandCategory::CapabilityOperand, Cap));
}
-void SPIRV::RequirementHandler::removeCapabilityIf(const Capability::Capability ToRemove,
- const Capability::Capability IfPresent) {
+void SPIRV::RequirementHandler::removeCapabilityIf(
+ const Capability::Capability ToRemove,
+ const Capability::Capability IfPresent) {
if (AvailableCaps.contains(IfPresent))
AvailableCaps.erase(ToRemove);
}
>From 61fd18c444d1e5ea59b4f7d1346c38158b6ab803 Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Fri, 22 Sep 2023 12:47:44 +0200
Subject: [PATCH 6/7] Rerun clang-format
---
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index 72162204afc823b..66deb6fe42fcb83 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -526,7 +526,7 @@ void SPIRV::RequirementHandler::addAvailableCaps(const CapabilityList &ToAdd) {
}
void SPIRV::RequirementHandler::removeCapabilityIf(
- const Capability::Capability ToRemove,
+ const Capability::Capability ToRemove,
const Capability::Capability IfPresent) {
if (AvailableCaps.contains(IfPresent))
AvailableCaps.erase(ToRemove);
@@ -910,7 +910,7 @@ void addInstrRequirements(const MachineInstr &MI,
// If we require capability Shader, then we can remove the requirement for
// the BitInstructions capability, since Shader is a superset capability
// of BitInstructions.
- Reqs.removeCapabilityIf(SPIRV::Capability::BitInstructions,
+ Reqs.removeCapabilityIf(SPIRV::Capability::BitInstructions,
SPIRV::Capability::Shader);
}
>From 6663b35fbba62ba085b28e258b4b29af41b85450 Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Fri, 22 Sep 2023 14:42:40 +0200
Subject: [PATCH 7/7] Remove extra 'the' in the option description
---
llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
index 1584ed477fa4ff2..e4ad805f071b9a7 100644
--- a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp
@@ -45,7 +45,7 @@ cl::list<SPIRV::Extension::Extension> Extensions(
clEnumValN(SPIRV::Extension::SPV_KHR_bit_instructions,
"SPV_KHR_bit_instructions",
"This enables bit instructions to be used by SPIR-V modules "
- "without the requiring the Shader capability")));
+ "without requiring the Shader capability")));
// Compare version numbers, but allow 0 to mean unspecified.
static bool isAtLeastVer(uint32_t Target, uint32_t VerToCompareTo) {
More information about the llvm-commits
mailing list