[SPIRV] Promote scalar arguments to vector for OpExtInst in generateExtInst instead of SPIRVRegularizer
#170155
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jmmartinez
requested review from
AlexVlx,
MrSidims,
maarquitos14,
s-perron and
sarnex
Member
|
@llvm/pr-subscribers-backend-spir-v Author: Juan Manuel Martinez Caamaño (jmmartinez) ChangesThis patch consist of 2 parts:
The implementation in
This patch does the scalar to vector promotion just before the One change in the generated code from the previous implementation is that this version uses a single Full diff: https://github.com/llvm/llvm-project/pull/170155.diff 5 Files Affected:
diff --git a/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp b/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
index 87ebee6a14eac..6ae0106d0129b 100644
--- a/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVBuiltins.cpp
@@ -1157,6 +1157,29 @@ static unsigned getNumSizeComponents(SPIRVType *imgType) {
return arrayed ? numComps + 1 : numComps;
}
+static bool builtinMayNeedPromotionToVec(uint32_t BuiltinNumber) {
+ switch (BuiltinNumber) {
+ case SPIRV::OpenCLExtInst::s_min:
+ case SPIRV::OpenCLExtInst::u_min:
+ case SPIRV::OpenCLExtInst::s_max:
+ case SPIRV::OpenCLExtInst::u_max:
+ case SPIRV::OpenCLExtInst::fmax:
+ case SPIRV::OpenCLExtInst::fmin:
+ case SPIRV::OpenCLExtInst::fmax_common:
+ case SPIRV::OpenCLExtInst::fmin_common:
+ case SPIRV::OpenCLExtInst::s_clamp:
+ case SPIRV::OpenCLExtInst::fclamp:
+ case SPIRV::OpenCLExtInst::u_clamp:
+ case SPIRV::OpenCLExtInst::mix:
+ case SPIRV::OpenCLExtInst::step:
+ case SPIRV::OpenCLExtInst::smoothstep:
+ return true;
+ default:
+ break;
+ }
+ return false;
+}
+
//===----------------------------------------------------------------------===//
// Implementation functions for each builtin group
//===----------------------------------------------------------------------===//
@@ -1182,16 +1205,37 @@ static bool generateExtInst(const SPIRV::IncomingCall *Call,
: SPIRV::OpenCLExtInst::fmax;
}
+ Register ReturnTypeId = GR->getSPIRVTypeID(Call->ReturnType);
+ SmallVector<Register> Arguments;
+ unsigned ResultElementCount =
+ GR->getScalarOrVectorComponentCount(ReturnTypeId);
+ bool MayNeedPromotionToVec =
+ builtinMayNeedPromotionToVec(Number) && ResultElementCount > 1;
+ for (Register Argument : Call->Arguments) {
+ Register PromotedArg = Argument;
+ SPIRVType *ArgumentType = GR->getSPIRVTypeForVReg(Argument);
+ if (MayNeedPromotionToVec && ArgumentType != Call->ReturnType) {
+ PromotedArg = createVirtualRegister(Call->ReturnType, GR, MIRBuilder);
+ auto VecBroadcast = MIRBuilder.buildInstr(SPIRV::OpCompositeConstruct)
+ .addDef(PromotedArg)
+ .addUse(ReturnTypeId);
+ for (unsigned I = 0; I != ResultElementCount; ++I)
+ VecBroadcast.addUse(Argument);
+ }
+ Arguments.push_back(PromotedArg);
+ }
+
// Build extended instruction.
auto MIB =
MIRBuilder.buildInstr(SPIRV::OpExtInst)
.addDef(Call->ReturnRegister)
- .addUse(GR->getSPIRVTypeID(Call->ReturnType))
+ .addUse(ReturnTypeId)
.addImm(static_cast<uint32_t>(SPIRV::InstructionSet::OpenCL_std))
.addImm(Number);
- for (auto Argument : Call->Arguments)
+ for (Register Argument : Arguments)
MIB.addUse(Argument);
+
MIB.getInstr()->copyIRFlags(CB);
if (OrigNumber == SPIRV::OpenCLExtInst::fmin_common ||
OrigNumber == SPIRV::OpenCLExtInst::fmax_common) {
diff --git a/llvm/lib/Target/SPIRV/SPIRVRegularizer.cpp b/llvm/lib/Target/SPIRV/SPIRVRegularizer.cpp
index 1b95f09974c61..653c9ad53e888 100644
--- a/llvm/lib/Target/SPIRV/SPIRVRegularizer.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVRegularizer.cpp
@@ -12,11 +12,10 @@
//===----------------------------------------------------------------------===//
#include "SPIRV.h"
-#include "llvm/Demangle/Demangle.h"
+#include "llvm/IR/Constants.h"
#include "llvm/IR/InstIterator.h"
-#include "llvm/IR/InstVisitor.h"
+#include "llvm/IR/Instructions.h"
#include "llvm/IR/PassManager.h"
-#include "llvm/Transforms/Utils/Cloning.h"
#include <list>
@@ -25,9 +24,7 @@
using namespace llvm;
namespace {
-struct SPIRVRegularizer : public FunctionPass, InstVisitor<SPIRVRegularizer> {
- DenseMap<Function *, Function *> Old2NewFuncs;
-
+struct SPIRVRegularizer : public FunctionPass {
public:
static char ID;
SPIRVRegularizer() : FunctionPass(ID) {}
@@ -37,11 +34,8 @@ struct SPIRVRegularizer : public FunctionPass, InstVisitor<SPIRVRegularizer> {
void getAnalysisUsage(AnalysisUsage &AU) const override {
FunctionPass::getAnalysisUsage(AU);
}
- void visitCallInst(CallInst &CI);
private:
- void visitCallScalToVec(CallInst *CI, StringRef MangledName,
- StringRef DemangledName);
void runLowerConstExpr(Function &F);
};
} // namespace
@@ -157,98 +151,8 @@ void SPIRVRegularizer::runLowerConstExpr(Function &F) {
}
}
-// It fixes calls to OCL builtins that accept vector arguments and one of them
-// is actually a scalar splat.
-void SPIRVRegularizer::visitCallInst(CallInst &CI) {
- auto F = CI.getCalledFunction();
- if (!F)
- return;
-
- auto MangledName = F->getName();
- char *NameStr = itaniumDemangle(F->getName().data());
- if (!NameStr)
- return;
- StringRef DemangledName(NameStr);
-
- // TODO: add support for other builtins.
- if (DemangledName.starts_with("fmin") || DemangledName.starts_with("fmax") ||
- DemangledName.starts_with("min") || DemangledName.starts_with("max"))
- visitCallScalToVec(&CI, MangledName, DemangledName);
- free(NameStr);
-}
-
-void SPIRVRegularizer::visitCallScalToVec(CallInst *CI, StringRef MangledName,
- StringRef DemangledName) {
- // Check if all arguments have the same type - it's simple case.
- auto Uniform = true;
- Type *Arg0Ty = CI->getOperand(0)->getType();
- auto IsArg0Vector = isa<VectorType>(Arg0Ty);
- for (unsigned I = 1, E = CI->arg_size(); Uniform && (I != E); ++I)
- Uniform = isa<VectorType>(CI->getOperand(I)->getType()) == IsArg0Vector;
- if (Uniform)
- return;
-
- auto *OldF = CI->getCalledFunction();
- Function *NewF = nullptr;
- auto [It, Inserted] = Old2NewFuncs.try_emplace(OldF);
- if (Inserted) {
- AttributeList Attrs = CI->getCalledFunction()->getAttributes();
- SmallVector<Type *, 2> ArgTypes = {OldF->getArg(0)->getType(), Arg0Ty};
- auto *NewFTy =
- FunctionType::get(OldF->getReturnType(), ArgTypes, OldF->isVarArg());
- NewF = Function::Create(NewFTy, OldF->getLinkage(), OldF->getName(),
- *OldF->getParent());
- ValueToValueMapTy VMap;
- auto NewFArgIt = NewF->arg_begin();
- for (auto &Arg : OldF->args()) {
- auto ArgName = Arg.getName();
- NewFArgIt->setName(ArgName);
- VMap[&Arg] = &(*NewFArgIt++);
- }
- SmallVector<ReturnInst *, 8> Returns;
- CloneFunctionInto(NewF, OldF, VMap,
- CloneFunctionChangeType::LocalChangesOnly, Returns);
- NewF->setAttributes(Attrs);
- It->second = NewF;
- } else {
- NewF = It->second;
- }
- assert(NewF);
-
- // This produces an instruction sequence that implements a splat of
- // CI->getOperand(1) to a vector Arg0Ty. However, we use InsertElementInst
- // and ShuffleVectorInst to generate the same code as the SPIR-V translator.
- // For instance (transcoding/OpMin.ll), this call
- // call spir_func <2 x i32> @_Z3minDv2_ii(<2 x i32> <i32 1, i32 10>, i32 5)
- // is translated to
- // %8 = OpUndef %v2uint
- // %14 = OpConstantComposite %v2uint %uint_1 %uint_10
- // ...
- // %10 = OpCompositeInsert %v2uint %uint_5 %8 0
- // %11 = OpVectorShuffle %v2uint %10 %8 0 0
- // %call = OpExtInst %v2uint %1 s_min %14 %11
- auto ConstInt = ConstantInt::get(IntegerType::get(CI->getContext(), 32), 0);
- PoisonValue *PVal = PoisonValue::get(Arg0Ty);
- Instruction *Inst = InsertElementInst::Create(
- PVal, CI->getOperand(1), ConstInt, "", CI->getIterator());
- ElementCount VecElemCount = cast<VectorType>(Arg0Ty)->getElementCount();
- Constant *ConstVec = ConstantVector::getSplat(VecElemCount, ConstInt);
- Value *NewVec =
- new ShuffleVectorInst(Inst, PVal, ConstVec, "", CI->getIterator());
- CI->setOperand(1, NewVec);
- CI->replaceUsesOfWith(OldF, NewF);
- CI->mutateFunctionType(NewF->getFunctionType());
-}
-
bool SPIRVRegularizer::runOnFunction(Function &F) {
runLowerConstExpr(F);
- visit(F);
- for (auto &OldNew : Old2NewFuncs) {
- Function *OldF = OldNew.first;
- Function *NewF = OldNew.second;
- NewF->takeName(OldF);
- OldF->eraseFromParent();
- }
return true;
}
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpExtInst_vector_promoton.ll b/llvm/test/CodeGen/SPIRV/transcoding/OpExtInst_vector_promoton.ll
new file mode 100644
index 0000000000000..9867b274d761c
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/transcoding/OpExtInst_vector_promoton.ll
@@ -0,0 +1,179 @@
+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+;
+; Some OpenCL builtins have mixed vector-scalar variants, but OpExtInt only supports
+; versions where all the arguments have the same type.
+;
+; We generate code, but it is invalid
+; We should generate vector versions for these cases
+
+define spir_kernel void @S_MIN() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function S_MIN
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: %[[VEC:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR:[0-9]+]] %[[SCALAR]]
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %[[VECTYPE]] %{{[0-9]+}} s_min %{{[0-9]+}} %[[VEC]]
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ %call = tail call spir_func <2 x i32> @_Z3minDv2_ii(<2 x i32> <i32 1, i32 10>, i32 5)
+ ret void
+}
+
+define spir_kernel void @U_MIN() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function U_MIN
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: %[[VEC:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR:[0-9]+]] %[[SCALAR]]
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %[[VECTYPE]] %{{[0-9]+}} u_min %{{[0-9]+}} %[[VEC]]
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ %call = tail call spir_func <2 x i32> @_Z3minDv2_jj(<2 x i32> <i32 1, i32 10>, i32 5)
+ ret void
+}
+
+define spir_kernel void @S_MAX() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function S_MAX
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: %[[VEC:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR:[0-9]+]] %[[SCALAR]]
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %[[VECTYPE]] %{{[0-9]+}} s_max %{{[0-9]+}} %[[VEC]]
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ %call = tail call spir_func <2 x i32> @_Z3maxDv2_ii(<2 x i32> <i32 1, i32 10>, i32 5)
+ ret void
+}
+
+define spir_kernel void @F_MIN() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function F_MIN
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: %[[VEC:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR:[0-9]+]] %[[SCALAR]]
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %[[VECTYPE]] %{{[0-9]+}} fmin %{{[0-9]+}} %[[VEC]]
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ %call = tail call spir_func <2 x float> @_Z3minDv2_ff(<2 x float> <float 1.0, float 10.0>, float 5.0)
+ ret void
+}
+
+define spir_kernel void @F_MAX() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function F_MAX
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: %[[VEC:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR:[0-9]+]] %[[SCALAR]]
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %[[VECTYPE]] %{{[0-9]+}} fmax %{{[0-9]+}} %[[VEC]]
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ %call = tail call spir_func <2 x float> @_Z3maxDv2_ff(<2 x float> <float 1.0, float 10.0>, float 5.0)
+ ret void
+}
+
+define spir_kernel void @F_FMIN() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function F_FMIN
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: %[[VEC:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR:[0-9]+]] %[[SCALAR]]
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %[[VECTYPE]] %{{[0-9]+}} fmin %{{[0-9]+}} %[[VEC]]
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ %call = tail call spir_func <2 x float> @_Z4fminDv2_ff(<2 x float> <float 1.0, float 10.0>, float 5.0)
+ ret void
+}
+
+define spir_kernel void @F_FMAX() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function F_FMAX
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: %[[VEC:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR:[0-9]+]] %[[SCALAR]]
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %[[VECTYPE]] %{{[0-9]+}} fmax %{{[0-9]+}} %[[VEC]]
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ %call = tail call spir_func <2 x float> @_Z4fmaxDv2_ff(<2 x float> <float 1.0, float 10.0>, float 5.0)
+ ret void
+}
+
+define spir_kernel void @S_CLAMP() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function S_CLAMP
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: %[[VEC_0:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR_0:[0-9]+]] %[[SCALAR_0]]
+; CHECK-NEXT: %[[VEC_1:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR_1:[0-9]+]] %[[SCALAR_1]]
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %[[VECTYPE]] %{{[0-9]+}} s_clamp %{{[0-9]+}} %[[VEC_0]] %[[VEC_1]]
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ %call = tail call spir_func <2 x i32> @_Z5clampDv2_iii(<2 x i32> <i32 1, i32 10>, i32 5, i32 6)
+ ret void
+}
+
+define spir_kernel void @F_CLAMP() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function F_CLAMP
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: %[[VEC_0:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR_0:[0-9]+]] %[[SCALAR_0]]
+; CHECK-NEXT: %[[VEC_1:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR_1:[0-9]+]] %[[SCALAR_1]]
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %[[VECTYPE]] %{{[0-9]+}} fclamp %{{[0-9]+}} %[[VEC_0]] %[[VEC_1]]
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ %call = tail call spir_func <2 x float> @_Z5clampDv2_fff(<2 x float> <float 1.0, float 10.0>, float 5.0, float 6.0)
+ ret void
+}
+
+define spir_kernel void @MIX() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function MIX
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: %[[VEC:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR:[0-9]+]] %[[SCALAR]]
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %[[VECTYPE]] %{{[0-9]+}} mix %{{[0-9]+}} %{{[0-9]+}} %[[VEC]]
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ %call = tail call spir_func <2 x float> @_Z3mixDv2_fS_f(<2 x float> <float 1.0, float 10.0>, <2 x float> <float 2.0, float 20.0>, float 0.5)
+ ret void
+}
+
+define spir_kernel void @SMOOTHSTEP() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function SMOOTHSTEP
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: %[[VEC_0:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR_0:[0-9]+]] %[[SCALAR_0]]
+; CHECK-NEXT: %[[VEC_1:[0-9]+]] = OpCompositeConstruct %[[VECTYPE:[0-9]+]] %[[SCALAR_1:[0-9]+]] %[[SCALAR_1]]
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %[[VECTYPE]] %{{[0-9]+}} smoothstep %[[VEC_0]] %[[VEC_1]] %{{[0-9]+}}
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ %call = tail call spir_func <2 x float> @_Z10smoothstepffDv2_f(float 1.0, float 0.5, <2 x float> <float 1.0, float 10.0>)
+ ret void
+}
+
+define spir_kernel void @ill_0() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function ill_0
+; CHECK-NEXT: OpLabel
+; CHECK-NEXT: OpFunctionCall %{{[0-9]+}} %{{[0-9]+}}
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ tail call spir_func void @_Z3minv()
+ ret void
+}
+
+declare spir_func <2 x i32> @_Z3minDv2_ii(<2 x i32>, i32)
+declare spir_func <2 x i32> @_Z3minDv2_jj(<2 x i32>, i32)
+declare spir_func <2 x i32> @_Z3maxDv2_ii(<2 x i32>, i32)
+declare spir_func <2 x float> @_Z3minDv2_ff(<2 x float>, float)
+declare spir_func <2 x float> @_Z3maxDv2_ff(<2 x float>, float)
+declare spir_func <2 x float> @_Z4fminDv2_ff(<2 x float>, float)
+declare spir_func <2 x float> @_Z4fmaxDv2_ff(<2 x float>, float)
+declare spir_func <2 x i32> @_Z5clampDv2_iii(<2 x i32>, i32)
+declare spir_func <2 x float> @_Z5clampDv2_fff(<2 x float>, float)
+declare spir_func <2 x float> @_Z3mixDv2_fS_f(<2 x float>, <2 x float>, float)
+declare spir_func <2 x float> @_Z10smoothstepffDv2_f(float, float, <2 x float>)
+declare spir_func void @_Z3minv()
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpExtInst_vector_promoton_bug.ll b/llvm/test/CodeGen/SPIRV/transcoding/OpExtInst_vector_promoton_bug.ll
new file mode 100644
index 0000000000000..5037d15e65cde
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/transcoding/OpExtInst_vector_promoton_bug.ll
@@ -0,0 +1,20 @@
+; XFAIL: *
+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+;
+; _Z3miniii is not a valid OpenCL intrinsic, do not treat it like one.
+
+define spir_kernel void @ill_1() {
+; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function ill_1
+; CHECK-NEXT: OpLabel
+; This is wrong, we should generate a regular call
+; CHECK-NEXT: %{{[0-9]+}} = OpExtInst %{{[0-9]+}} %{{[0-9]+}} s_min %{{[0-9]+}} %{{[0-9]+}} %{{[0-9]+}}
+; CHECK-NEXT: OpReturn
+; CHECK-NEXT: OpFunctionEnd
+; CHECK-NEXT: ; -- End function
+entry:
+ tail call spir_func void @_Z3miniii(i32 1, i32 2, i32 3)
+ ret void
+}
+
+declare spir_func i32 @_Z3miniii(i32, i32, i32)
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpMin.ll b/llvm/test/CodeGen/SPIRV/transcoding/OpMin.ll
deleted file mode 100644
index 5cc3ea01e5191..0000000000000
--- a/llvm/test/CodeGen/SPIRV/transcoding/OpMin.ll
+++ /dev/null
@@ -1,16 +0,0 @@
-; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
-
-; CHECK-SPIRV: %[[#SetInstID:]] = OpExtInstImport "OpenCL.std"
-; CHECK-SPIRV: %[[#IntTypeID:]] = OpTypeInt 32 [[#]]
-; CHECK-SPIRV: %[[#Int2TypeID:]] = OpTypeVector %[[#IntTypeID]] 2
-; CHECK-SPIRV: %[[#CompositeID:]] = OpCompositeInsert %[[#Int2TypeID]] %[[#]] %[[#]] [[#]]
-; CHECK-SPIRV: %[[#ShuffleID:]] = OpVectorShuffle %[[#Int2TypeID]] %[[#CompositeID]] %[[#]] [[#]] [[#]]
-; CHECK-SPIRV: %[[#]] = OpExtInst %[[#Int2TypeID]] %[[#SetInstID]] s_min %[[#]] %[[#ShuffleID]]
-
-define spir_kernel void @test() {
-entry:
- %call = tail call spir_func <2 x i32> @_Z3minDv2_ii(<2 x i32> <i32 1, i32 10>, i32 5) #2
- ret void
-}
-
-declare spir_func <2 x i32> @_Z3minDv2_ii(<2 x i32>, i32)
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
s-perron
reviewed
Dec 2, 2025
Contributor
s-perron
left a comment
s-perron
left a comment
There was a problem hiding this comment.
Look reasonable to me. Just a couple style issues for clarity.
jmmartinez
force-pushed
the
users/jmmartinez/spirv/opencl_intrinsic_opextinst_rewrite
branch
from
b0a276a to
f01c0e9
Compare
Contributor
Author
|
Thanks for the comments ! |
maarquitos14
reviewed
Dec 9, 2025
Contributor
maarquitos14
left a comment
maarquitos14
left a comment
There was a problem hiding this comment.
LGTM, just a few nits. Also, the two new test files have a typo, I think: promoton when it should be promotion.
llvm/test/CodeGen/SPIRV/transcoding/OpExtInst_vector_promoton.ll
Outdated
Show resolved
Hide resolved
llvm/test/CodeGen/SPIRV/transcoding/OpExtInst_vector_promoton.ll
Outdated
Show resolved
Hide resolved
| define spir_kernel void @ill_1() { | ||
| ; CHECK-LABEL: OpFunction %{{[0-9]+}} None %{{[0-9]+}} ; -- Begin function ill_1 | ||
| ; CHECK-NEXT: OpLabel | ||
| ; This is wrong, we should generate a regular call |
Contributor
There was a problem hiding this comment.
So with this patch this still fails?
Contributor
Author
There was a problem hiding this comment.
Yes. This problem is orthogonal to the one addressed in the PR.
The backend's builtin recognition matches builtins using the demangled function name, the first argument and eventually the last argument. But not the exact mangled function name.
min(int, int, int) has the same demangled name and first argument type as min(int int), so both match the s_min OpenCL builtin.
jmmartinez
force-pushed
the
users/jmmartinez/spirv/opencl_intrinsic_opextinst_rewrite
branch
from
47205d0 to
a2b1bda
Compare
maarquitos14
approved these changes
Dec 10, 2025
jmmartinez
deleted the
users/jmmartinez/spirv/opencl_intrinsic_opextinst_rewrite
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch consist of 2 parts:
SPIRVRegularizer;generateExtInst.The implementation in
SPIRVRegularizerhad several issues:This patch does the scalar to vector promotion just before the
OpExtInstis generated. Without relying on the IR transformation.One change in the generated code from the previous implementation is that this version uses a single
OpCompositeConstructoperation to convert the scalar into a vector. The old implementation inserted an element at the 0 position in anundefvector (usingOpCompositeInsert); then copied that element for every vector element usingOpVectorShuffle.This patch also adds a test (
OpExtInst_vector_promoton_bug.ll) that highlights an issue in the builtin pattern matching that we're using: our pattern matching doesn't consider the number of arguments, only the demangled name, first and last arguments.