-
Notifications
You must be signed in to change notification settings - Fork 221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPIR-V Extension] fpbuiltin-max-error support #2056
Changes from all commits
3564ad5
8d9ef15
13df725
132c531
e1d0775
340d530
66f1cd6
20bd965
22f367b
c20e426
1f7a4ab
5df3983
bc3eeac
eaa8573
b4c5b7a
1303aa5
95f0190
833bd78
2f40dbf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,6 +106,19 @@ using namespace llvm; | |
using namespace SPIRV; | ||
using namespace OCLUtil; | ||
|
||
namespace { | ||
|
||
static SPIRVWord convertFloatToSPIRVWord(float F) { | ||
union { | ||
float F; | ||
SPIRVWord Spir; | ||
} FPMaxError; | ||
FPMaxError.F = F; | ||
return FPMaxError.Spir; | ||
} | ||
|
||
} // namespace | ||
|
||
namespace SPIRV { | ||
|
||
static void foreachKernelArgMD( | ||
|
@@ -3481,6 +3494,26 @@ bool LLVMToSPIRVBase::isKnownIntrinsic(Intrinsic::ID Id) { | |
} | ||
} | ||
|
||
// Add decoration if needed | ||
SPIRVInstruction *addFPBuiltinDecoration(SPIRVModule *BM, IntrinsicInst *II, | ||
SPIRVInstruction *I) { | ||
const bool AllowFPMaxError = | ||
BM->isAllowedToUseExtension(ExtensionID::SPV_INTEL_fp_max_error); | ||
assert(II->getCalledFunction()->getName().startswith("llvm.fpbuiltin")); | ||
// Add a new decoration for llvm.builtin intrinsics, if needed | ||
if (AllowFPMaxError) | ||
if (II->getAttributes().hasFnAttr("fpbuiltin-max-error")) { | ||
double F = 0.0; | ||
II->getAttributes() | ||
.getFnAttr("fpbuiltin-max-error") | ||
.getValueAsString() | ||
.getAsDouble(F); | ||
I->addDecorate(DecorationFPMaxErrorDecorationINTEL, | ||
convertFloatToSPIRVWord(F)); | ||
} | ||
return I; | ||
} | ||
|
||
// Performs mapping of LLVM IR rounding mode to SPIR-V rounding mode | ||
// Value *V is metadata <rounding mode> argument of | ||
// llvm.experimental.constrained.* intrinsics | ||
|
@@ -4424,8 +4457,9 @@ SPIRVValue *LLVMToSPIRVBase::transIntrinsicInst(IntrinsicInst *II, | |
} | ||
return Result; | ||
} | ||
|
||
default: | ||
if (auto *BVar = transFPBuiltinIntrinsicInst(II, BB)) | ||
return BVar; | ||
if (BM->isUnknownIntrinsicAllowed(II)) | ||
return BM->addCallInst( | ||
transFunctionDecl(II->getCalledFunction()), | ||
|
@@ -4441,6 +4475,124 @@ SPIRVValue *LLVMToSPIRVBase::transIntrinsicInst(IntrinsicInst *II, | |
return nullptr; | ||
} | ||
|
||
LLVMToSPIRVBase::FPBuiltinType | ||
LLVMToSPIRVBase::getFPBuiltinType(IntrinsicInst *II, StringRef &OpName) { | ||
StringRef Name = II->getCalledFunction()->getName(); | ||
if (!Name.startswith("llvm.fpbuiltin")) | ||
return FPBuiltinType::UNKNOWN; | ||
Name.consume_front("llvm.fpbuiltin."); | ||
OpName = Name.split('.').first; | ||
FPBuiltinType Type = | ||
StringSwitch<FPBuiltinType>(OpName) | ||
.Cases("fadd", "fsub", "fmul", "fdiv", "frem", | ||
FPBuiltinType::REGULAR_MATH) | ||
.Cases("sin", "cos", "tan", FPBuiltinType::EXT_1OPS) | ||
.Cases("sinh", "cosh", "tanh", FPBuiltinType::EXT_1OPS) | ||
.Cases("asin", "acos", "atan", FPBuiltinType::EXT_1OPS) | ||
.Cases("asinh", "acosh", "atanh", FPBuiltinType::EXT_1OPS) | ||
.Cases("exp", "exp2", "exp10", "expm1", FPBuiltinType::EXT_1OPS) | ||
.Cases("log", "log2", "log10", "log1p", FPBuiltinType::EXT_1OPS) | ||
.Cases("sqrt", "rsqrt", "erf", "erfc", FPBuiltinType::EXT_1OPS) | ||
.Cases("atan2", "pow", "hypot", "ldexp", FPBuiltinType::EXT_2OPS) | ||
.Case("sincos", FPBuiltinType::EXT_3OPS) | ||
.Default(FPBuiltinType::UNKNOWN); | ||
return Type; | ||
} | ||
|
||
SPIRVValue *LLVMToSPIRVBase::transFPBuiltinIntrinsicInst(IntrinsicInst *II, | ||
SPIRVBasicBlock *BB) { | ||
StringRef OpName; | ||
auto FPBuiltinTypeVal = getFPBuiltinType(II, OpName); | ||
if (FPBuiltinTypeVal == FPBuiltinType::UNKNOWN) | ||
return nullptr; | ||
switch (FPBuiltinTypeVal) { | ||
case FPBuiltinType::REGULAR_MATH: { | ||
auto BinOp = StringSwitch<Op>(OpName) | ||
.Case("fadd", OpFAdd) | ||
.Case("fsub", OpFSub) | ||
.Case("fmul", OpFMul) | ||
.Case("fdiv", OpFDiv) | ||
.Case("frem", OpFRem) | ||
.Default(OpUndef); | ||
auto *BI = BM->addBinaryInst(BinOp, transType(II->getType()), | ||
transValue(II->getArgOperand(0), BB), | ||
transValue(II->getArgOperand(1), BB), BB); | ||
return addFPBuiltinDecoration(BM, II, BI); | ||
} | ||
case FPBuiltinType::EXT_1OPS: { | ||
if (!checkTypeForSPIRVExtendedInstLowering(II, BM)) | ||
break; | ||
SPIRVType *STy = transType(II->getType()); | ||
std::vector<SPIRVValue *> Ops(1, transValue(II->getArgOperand(0), BB)); | ||
auto ExtOp = StringSwitch<SPIRVWord>(OpName) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it suppose to work for ESIMD SYCL programming model? I'm asking because not all backends support OpenCL ext instructions There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do expect the backends to atleast support the subset of instructions that are shown here. Thanks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @vmustya just checking your opinion There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently IGC VC backend only supports the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @asudarsa to consider changing math ext instructions to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @andykaylor I'm talking about OpenCL builtins described here: https://registry.khronos.org/SPIR-V/specs/1.0/OpenCL.ExtendedInstructionSet.100.mobile.html native vs non-native. AFAIK IGC scalar support all of the builtins, while vector compiler support only 'native'. I just want to ensure, that we are on the same page and understand consequences of merging implementation going through non-native builtins. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Performance-wise: that's what the spec says: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MrSidims My concern is for the case where we're trying to restrict accuracy beyond the normal SYCL requirements. For example, the cos() function normally only requires 4 ulp accuracy, but I might want to call it with a 1 ulp accuracy requirement. My understanding of the native_ OCL instructions is that native instructions may be used regardless of their accuracy. So if we're trying to require 1 ulp accuracy, using the native_ instructions isn't appropriate. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @andykaylor thanks for the explanation! I just wanted to ensure, that we understand that we sacrifice portability (at least temporary) and have a reasoning for it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MrSidims Yes, sacrificing portability when the accuracy controls are used is expected. I expect that the accuracy controls will only be used by advanced users who are trying to fine-tune their implementations. I hope that if the feature is successful more vendors will add support for it and the portability problem will be resolved. |
||
.Case("sin", OpenCLLIB::Sin) | ||
.Case("cos", OpenCLLIB::Cos) | ||
.Case("tan", OpenCLLIB::Tan) | ||
.Case("sinh", OpenCLLIB::Sinh) | ||
.Case("cosh", OpenCLLIB::Cosh) | ||
.Case("tanh", OpenCLLIB::Tanh) | ||
.Case("asin", OpenCLLIB::Asin) | ||
.Case("acos", OpenCLLIB::Acos) | ||
.Case("atan", OpenCLLIB::Atan) | ||
.Case("asinh", OpenCLLIB::Asinh) | ||
.Case("acosh", OpenCLLIB::Acosh) | ||
.Case("atanh", OpenCLLIB::Atanh) | ||
.Case("exp", OpenCLLIB::Exp) | ||
.Case("exp2", OpenCLLIB::Exp2) | ||
.Case("exp10", OpenCLLIB::Exp10) | ||
.Case("expm1", OpenCLLIB::Expm1) | ||
.Case("log", OpenCLLIB::Log) | ||
.Case("log2", OpenCLLIB::Log2) | ||
.Case("log10", OpenCLLIB::Log10) | ||
.Case("log1p", OpenCLLIB::Log1p) | ||
.Case("sqrt", OpenCLLIB::Sqrt) | ||
.Case("rsqrt", OpenCLLIB::Rsqrt) | ||
.Case("erf", OpenCLLIB::Erf) | ||
.Case("erfc", OpenCLLIB::Erfc) | ||
.Default(SPIRVWORD_MAX); | ||
asudarsa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
assert(ExtOp != SPIRVWORD_MAX); | ||
auto *BI = BM->addExtInst(STy, BM->getExtInstSetId(SPIRVEIS_OpenCL), ExtOp, | ||
Ops, BB); | ||
return addFPBuiltinDecoration(BM, II, BI); | ||
} | ||
case FPBuiltinType::EXT_2OPS: { | ||
if (!checkTypeForSPIRVExtendedInstLowering(II, BM)) | ||
break; | ||
SPIRVType *STy = transType(II->getType()); | ||
std::vector<SPIRVValue *> Ops{transValue(II->getArgOperand(0), BB), | ||
transValue(II->getArgOperand(1), BB)}; | ||
auto ExtOp = StringSwitch<SPIRVWord>(OpName) | ||
.Case("atan2", OpenCLLIB::Atan2) | ||
.Case("hypot", OpenCLLIB::Hypot) | ||
.Case("pow", OpenCLLIB::Pow) | ||
.Case("ldexp", OpenCLLIB::Ldexp) | ||
.Default(SPIRVWORD_MAX); | ||
assert(ExtOp != SPIRVWORD_MAX); | ||
auto *BI = BM->addExtInst(STy, BM->getExtInstSetId(SPIRVEIS_OpenCL), ExtOp, | ||
Ops, BB); | ||
return addFPBuiltinDecoration(BM, II, BI); | ||
} | ||
case FPBuiltinType::EXT_3OPS: { | ||
if (!checkTypeForSPIRVExtendedInstLowering(II, BM)) | ||
break; | ||
SPIRVType *STy = transType(II->getType()); | ||
std::vector<SPIRVValue *> Ops{transValue(II->getArgOperand(0), BB), | ||
transValue(II->getArgOperand(1), BB), | ||
transValue(II->getArgOperand(2), BB)}; | ||
auto ExtOp = StringSwitch<SPIRVWord>(OpName) | ||
.Case("sincos", OpenCLLIB::Sincos) | ||
.Default(SPIRVWORD_MAX); | ||
assert(ExtOp != SPIRVWORD_MAX); | ||
auto *BI = BM->addExtInst(STy, BM->getExtInstSetId(SPIRVEIS_OpenCL), ExtOp, | ||
Ops, BB); | ||
return addFPBuiltinDecoration(BM, II, BI); | ||
} | ||
default: | ||
return nullptr; | ||
} | ||
return nullptr; | ||
} | ||
|
||
SPIRVValue *LLVMToSPIRVBase::transFenceInst(FenceInst *FI, | ||
SPIRVBasicBlock *BB) { | ||
SPIRVWord MemorySemantics; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
9b527c0fb60124936d0906d44803bec51a0200fb | ||
51b106461707f46d962554efe1bf56dee28958a3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just minor suggestion to wrap this to a separate function - to have more consistent code in
transDecoration()