-
Notifications
You must be signed in to change notification settings - Fork 239
Add emulation of (s/u)_(add/sub)_sat intrinsics if needed #2138
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -3860,21 +3860,130 @@ SPIRVValue *LLVMToSPIRVBase::transIntrinsicInst(IntrinsicInst *II, | |
case Intrinsic::usub_sat: | ||
case Intrinsic::sadd_sat: | ||
case Intrinsic::ssub_sat: { | ||
SPIRVWord ExtOp; | ||
if (IID == Intrinsic::uadd_sat) | ||
ExtOp = OpenCLLIB::UAdd_sat; | ||
else if (IID == Intrinsic::usub_sat) | ||
ExtOp = OpenCLLIB::USub_sat; | ||
else if (IID == Intrinsic::sadd_sat) | ||
ExtOp = OpenCLLIB::SAdd_sat; | ||
else | ||
ExtOp = OpenCLLIB::SSub_sat; | ||
if (BM->shouldUseOpenCLExtInstructionsForLLVMMathIntrinsic()) { | ||
SPIRVWord ExtOp; | ||
if (IID == Intrinsic::uadd_sat) | ||
ExtOp = OpenCLLIB::UAdd_sat; | ||
else if (IID == Intrinsic::usub_sat) | ||
ExtOp = OpenCLLIB::USub_sat; | ||
else if (IID == Intrinsic::sadd_sat) | ||
ExtOp = OpenCLLIB::SAdd_sat; | ||
else | ||
ExtOp = OpenCLLIB::SSub_sat; | ||
|
||
SPIRVType *Ty = transType(II->getType()); | ||
std::vector<SPIRVValue *> Operands = {transValue(II->getArgOperand(0), BB), | ||
transValue(II->getArgOperand(1), BB)}; | ||
return BM->addExtInst(Ty, BM->getExtInstSetId(SPIRVEIS_OpenCL), ExtOp, | ||
std::move(Operands), BB); | ||
SPIRVType *Ty = transType(II->getType()); | ||
std::vector<SPIRVValue *> Operands = { | ||
transValue(II->getArgOperand(0), BB), | ||
transValue(II->getArgOperand(1), BB)}; | ||
return BM->addExtInst(Ty, BM->getExtInstSetId(SPIRVEIS_OpenCL), ExtOp, | ||
std::move(Operands), BB); | ||
} | ||
|
||
Type *LLVMTy = II->getType(); | ||
SPIRVType *Ty = transType(LLVMTy); | ||
Type *BoolTy = IntegerType::getInt1Ty(M->getContext()); | ||
if (auto *VecTy = dyn_cast<VectorType>(LLVMTy)) | ||
BoolTy = VectorType::get(BoolTy, VecTy->getElementCount()); | ||
SPIRVType *SPVBoolTy = transType(BoolTy); | ||
SPIRVValue *FirstArgVal = transValue(II->getArgOperand(0), BB); | ||
SPIRVValue *SecondArgVal = transValue(II->getArgOperand(1), BB); | ||
MrSidims marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (IID == Intrinsic::uadd_sat) { | ||
// uadd.sat(a, b) -> res = a + b, (res >= a) ? res : MAX | ||
SPIRVValue *Max = | ||
transValue(Constant::getAllOnesValue(II->getType()), BB); | ||
SPIRVValue *Add = | ||
BM->addBinaryInst(OpIAdd, Ty, FirstArgVal, SecondArgVal, BB); | ||
SPIRVValue *Cmp = | ||
BM->addCmpInst(OpUGreaterThanEqual, SPVBoolTy, Add, FirstArgVal, BB); | ||
return BM->addSelectInst(Cmp, Add, Max, BB); | ||
} | ||
if (IID == Intrinsic::usub_sat) { | ||
// usub.sat(a, b) -> (a > b) ? a - b : 0 | ||
SPIRVValue *Sub = | ||
BM->addBinaryInst(OpISub, Ty, FirstArgVal, SecondArgVal, BB); | ||
SPIRVValue *Cmp = BM->addCmpInst(OpUGreaterThan, SPVBoolTy, FirstArgVal, | ||
SecondArgVal, BB); | ||
SPIRVValue *Zero = transValue(Constant::getNullValue(II->getType()), BB); | ||
return BM->addSelectInst(Cmp, Sub, Zero, BB); | ||
} | ||
|
||
uint64_t NumBits = LLVMTy->getScalarSizeInBits(); | ||
SPIRVValue *Zero = transValue(Constant::getNullValue(II->getType()), BB); | ||
SPIRVValue *Max = transValue( | ||
Constant::getIntegerValue(LLVMTy, APInt::getSignedMaxValue(NumBits)), | ||
BB); | ||
SPIRVValue *Min = transValue( | ||
Constant::getIntegerValue(LLVMTy, APInt::getSignedMinValue(NumBits)), | ||
BB); | ||
SPIRVValue *IsPositive = | ||
BM->addCmpInst(OpSGreaterThan, SPVBoolTy, SecondArgVal, Zero, BB); | ||
SPIRVValue *IsNegative = | ||
BM->addCmpInst(OpSLessThan, SPVBoolTy, SecondArgVal, Zero, BB); | ||
|
||
if (IID == Intrinsic::sadd_sat) { | ||
// sadd.sat(a, b) -> if (b > 0) && a > MAX - b => overflow -> MAX | ||
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. Can we use this sequence: res = a+b This makes it unnecessary to calculate "MAX - b" and "MIN - b" 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. Another implementation with a single comparison:
|
||
// -> else if (b < 0) && a < MIN - b => overflow -> MIN | ||
// -> else a + b | ||
// There two ways to represent such sequence in IR: | ||
// 1. Via 2 branch instructions plus 'phi' instruction; | ||
// 2. Via set of select instructions. | ||
// The later was chosed because of the following consideration: | ||
// speculative branch prediction will most likely consider 'if' statements | ||
// here as always false falling through to 'a + b', and reversing it will | ||
// cause performance degradation. | ||
SPIRVValue *Add = | ||
BM->addBinaryInst(OpIAdd, Ty, FirstArgVal, SecondArgVal, BB); | ||
|
||
// check if (b > 0) && a > MAX - b condition | ||
SPIRVValue *MaxSubB = | ||
BM->addBinaryInst(OpISub, Ty, Max, SecondArgVal, BB); | ||
SPIRVValue *CanPosOverflow = | ||
BM->addCmpInst(OpSGreaterThan, SPVBoolTy, FirstArgVal, MaxSubB, BB); | ||
SPIRVValue *PosOverflow = BM->addInstTemplate( | ||
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. Not sure if the order of the operand could impact performance here. Again just a request for quick check. Not a PR blocker. 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. TBH I'm not expecting this implementation be performant at all. Ideally we should generate such code that is collapsing to sadd/sub_sat intrinsic after we run InstCombine , but it's not happening. That is because InstCombine checks overflow by extending integer to a larger one and comparing the result with MAX/MIN of the previous type. We can't do this in general case, because we can't cast integer to a wider one in SPIR-V without extensions. |
||
OpLogicalAnd, {CanPosOverflow->getId(), IsPositive->getId()}, BB, | ||
SPVBoolTy); | ||
|
||
// check if (b < 0) && MIN - b > a condition | ||
SPIRVValue *MinSubB = | ||
BM->addBinaryInst(OpISub, Ty, Min, SecondArgVal, BB); | ||
SPIRVValue *CanNegOverflow = | ||
BM->addCmpInst(OpSGreaterThan, SPVBoolTy, MinSubB, FirstArgVal, BB); | ||
SPIRVValue *NegOverflow = BM->addInstTemplate( | ||
OpLogicalAnd, {CanNegOverflow->getId(), IsNegative->getId()}, BB, | ||
SPVBoolTy); | ||
|
||
// do selects | ||
SPIRVValue *FirstSelect = BM->addSelectInst(PosOverflow, Max, Add, BB); | ||
return BM->addSelectInst(NegOverflow, Min, FirstSelect, BB); | ||
} | ||
// ssub.sat(a, b) -> if (b > 0) && a < MIN + b => overflow -> MIN | ||
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. This can be: res = a-b This makes it unnecessary to calculate "MAX + b" and "MIN + b" |
||
// -> else if (b < 0) && a > MAX + b => overflow -> MAX | ||
// -> else a - b | ||
// still represent via 2 selects | ||
assert(IID == Intrinsic::ssub_sat && "Expected llvm.ssub_sat"); | ||
SPIRVValue *Sub = | ||
BM->addBinaryInst(OpISub, Ty, FirstArgVal, SecondArgVal, BB); | ||
|
||
// check if (b > 0) && MIN + b > a | ||
SPIRVValue *MinAddB = BM->addBinaryInst(OpIAdd, Ty, Min, SecondArgVal, BB); | ||
SPIRVValue *CanNegOverflow = | ||
BM->addCmpInst(OpSGreaterThan, SPVBoolTy, MinAddB, FirstArgVal, BB); | ||
SPIRVValue *NegOverflow = BM->addInstTemplate( | ||
OpLogicalAnd, {CanNegOverflow->getId(), IsPositive->getId()}, BB, | ||
SPVBoolTy); | ||
|
||
// check if (b < 0) && a > MAX + b | ||
SPIRVValue *MaxAddB = BM->addBinaryInst(OpIAdd, Ty, Max, SecondArgVal, BB); | ||
SPIRVValue *CanPosOverflow = | ||
BM->addCmpInst(OpSGreaterThan, SPVBoolTy, FirstArgVal, MaxAddB, BB); | ||
SPIRVValue *PosOverflow = BM->addInstTemplate( | ||
OpLogicalAnd, {CanPosOverflow->getId(), IsNegative->getId()}, BB, | ||
SPVBoolTy); | ||
|
||
// do selects | ||
SPIRVValue *FirstSelect = BM->addSelectInst(PosOverflow, Max, Sub, BB); | ||
return BM->addSelectInst(NegOverflow, Min, FirstSelect, BB); | ||
} | ||
case Intrinsic::memset: { | ||
// Generally there is no direct mapping of memset to SPIR-V. But it turns | ||
|
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.
nit regarding
noexcept
:LLVM itself prohibits C++ exceptions and they are turned off in CMakeLists by compiler flag
-fno-exceptions
.If we use exceptions in translator, then it would be better to stop doing it and start using the compiler flag
-fno-exceptions
as well.