[llvm] [SPIRV] Use `Op[S|U]Dot` when possible for integer dot product (PR #115095)
Finn Plummer via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 20 11:40:47 PST 2024
================
@@ -1013,21 +1013,26 @@ static void AddDotProductRequirements(const MachineInstr &MI,
Reqs.addCapability(SPIRV::Capability::DotProduct);
const MachineRegisterInfo &MRI = MI.getMF()->getRegInfo();
- const MachineInstr *InstrPtr = &MI;
- assert(MI.getOperand(1).isReg() && "Unexpected operand in dot");
+ assert(MI.getOperand(2).isReg() && "Unexpected operand in dot");
+ const MachineInstr *InputInstr = MRI.getVRegDef(MI.getOperand(2).getReg());
+ assert(InputInstr->getOperand(1).isReg() &&
+ "Unexpected operand in dot input");
----------------
inbelic wrote:
Sure.
The previous implementation is incorrect as the return register of the dot op is always a scalar integer type. Instead we need to check the type of the input operands as this is what the capability is correctly checked on. I think the terminology of the `InputInstr` is confusing then, as we are just checking the input register type and not really switching which instruction we are checking. I will fix that up to help with code readability.
The reason this wasn't caught was because the `dot4add` intrinsics only has `i32` inputs and so the other code-paths were not tested.
Unfortunately, I checked-in untested code, so a better way forward would have been to only add the `DotProductInput4x8BitPacked` capability in the previous `dot4add` commits and then added the other ones here.
https://github.com/llvm/llvm-project/pull/115095
More information about the llvm-commits
mailing list