[PATCH] D94142: [IR] Allow scalable vectors in structs to support intrinsics returning multiple values.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/test/Other/scalable-vector-struct-intrinsic.ll:6
+; values from intrinsics.
+
+declare { <vscale x 2 x i32>, <vscale x 2 x i1> } @llvm.sadd.with.overflow.nxv2i32(<vscale x 2 x i32>, <vscale x 2 x i32>)
----------------
sdesmalen wrote:
> sdesmalen wrote:
> > I guess this patch allows:
> >   declare { <vscale x 2 x i32>, i32 } @foo()
> > 
> > Is that deliberate?
> With the example above I actually meant to point out the combination of a scalable-vector and a non-scalable type (i32). I'm not sure if this would still be a problem if the only nodes that can return such a struct are intrinsics. Probably SelectionDAGBuilder can just take return value K from the SDNode for the lowered intrinsic. Can you confirm this won't lead to any issues?
Allowing scalable and non-scalable to be mixed was intentional. I'm using it in D94286.

The one issue I did encounter so far is that computeValueVTs called from SelectionDAGBuilder tries to create a StructLayout to compute offsets which causing the warning from TypeSize to trigger. Most callers of computeValueVTs don't want the offsets so I bandaged over it by not creating the StructLayout if the offsets aren't needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94142