[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