Dot product bugfix to include more floating point types#1578
Dot product bugfix to include more floating point types#1578svenvh merged 3 commits intoKhronosGroup:mainfrom
Conversation
Slightly changed the check for Dot call to include more floating point types. Bugfix for previous change regarding integer dot product.
svenvh
left a comment
svenvh
left a comment
There was a problem hiding this comment.
Slightly changed the check for Dot call in OCLToSPIRV to include more floating point types, as some were omitted by mistake.
I suppose you mean that any floating point type should be handled as before? It would be great if you could add a test case that was translated incorrectly after #1174 and is fixed with this change.
lib/SPIRV/OCLToSPIRV.cpp
Outdated
| (CI.getOperand(0)->getType()->isFloatTy() || | ||
| CI.getOperand(1)->getType()->isDoubleTy())) { | ||
| (!CI.getOperand(0)->getType()->isVectorTy() && | ||
| !CI.getOperand(0)->getType()->isIntegerTy())) { |
There was a problem hiding this comment.
Wondering if just CI.getOperand(0)->getType()->isFPOrFPVectorTy() is sufficient?
There was a problem hiding this comment.
From what I understand this visitCallDot is meant to translate dot with scalar floating point arguments (but not vectors) into FMul. Because of that FPVectors shouldn't pass this check but instead stay as calls to dot.
There was a problem hiding this comment.
isFloatingPointTy()?
There was a problem hiding this comment.
Yeah, that might be a good idea
Added test case using the half dot to the bugfix.
|
I've added a test case which uses the dot call with scalar half arguments. This should be translated into FMul but because of my previous change it would be translated into an integer dot (DotKHR). This is what I was mainly aiming to fix with this update. |
Switched the visitCallDot check to use isFloatingPointTy for scalar floating point operands.
…#1578) Switched the visitCallDot check to use isFloatingPointTy for scalar floating point operands. Bugfix for previous change regarding integer dot product.
…#1578) Switched the visitCallDot check to use isFloatingPointTy for scalar floating point operands. Bugfix for previous change regarding integer dot product.
…#1578) Switched the visitCallDot check to use isFloatingPointTy for scalar floating point operands. Bugfix for previous change regarding integer dot product.
…#1578) Switched the visitCallDot check to use isFloatingPointTy for scalar floating point operands. Bugfix for previous change regarding integer dot product.
Switched the visitCallDot check to use isFloatingPointTy for scalar floating point operands. Bugfix for previous change regarding integer dot product. (cherry picked from commit 71e01b5) Co-authored-by: Jakub Czarnecki <jakub.czarnecki@intel.com>
4 participants
This is a bugfix for my previous change ( #1174 ) regarding the integer dot product translation.
Slightly changed the check for Dot call in OCLToSPIRV to include more floating point types, as some were omitted by mistake.