[PATCH] D77129: [Verifier] Verify matrix dimensions operands match vector size.

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/IR/Verifier.cpp:4805
+      NumColumns = cast<ConstantInt>(Call.getArgOperand(4));
+      TypeToCheck = cast<VectorType>(Call.getType());
+      break;
----------------
fhahn wrote:
> SjoerdMeijer wrote:
> > fhahn wrote:
> > > SjoerdMeijer wrote:
> > > > Quick query on this and the semantics:
> > > > 
> > > >   declare vectorty @llvm.matrix.multiply.*(vectorty %A, vectorty %B, i32 <OuterRows>, i32 <Inner>, i32 <OuterColumns>)
> > > > 
> > > > do we expect the element types of vectors %A and %B to be same, and do we need to check this?
> > > Yes, the element types of all types must match currently, but I think it is neither checked in the verifier nor explicit in the LangRef. 
> > > 
> > > To generate code for llvm.aarch64.neon.udot & co, there probably needs to be a way to have different element type widths for result and source operands.
> > > Yes, the element types of all types must match currently, but I think it is neither checked in the verifier nor explicit in the LangRef.
> > 
> > I started looking at the matrix support, getting up to speed with it, and this is where I started and the first thing I noticed. Was just asking about that here as a sanity check. I wouldn't mind putting up a patch for that if that's helpful. Probably the least we can do for not is to check if we are not mixing integers and float types, and then we also need to add that to LangRef and be explicit about that.
> > I wouldn't mind putting up a patch for that if that's helpful.
> 
> That would be great. I think things will fall apart/miscompile if the element types differ at the moment.
cool, will do.

FYI: I started with committing a NFC patch adding some more negative tests: rG0fc17e9edc8f




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77129/new/

https://reviews.llvm.org/D77129