[PATCH] D128975: [AArc64] Add support for insert/extract for nxv1i1 types.

Mon Jul 4 07:39:25 PDT 2022

sdesmalen marked 3 inline comments as done.
sdesmalen added inline comments.

================
Comment at: llvm/test/CodeGen/AArch64/sve-insert-vector.ll:711

+;
+; Insert nxv1i1 type into: nxv2i1
----------------
david-arm wrote:
> Is there value in adding a simple test that inserts <vscale x 1 x i1> into a <vscale x 1 x i1> vector? I assume that standard DAG combines will treat this as a simple copy?
I think there's little value in that because it becomes a copy straight away when building the DAG.

================
Comment at: llvm/test/CodeGen/AArch64/sve-insert-vector.ll:992
+; CHECK-NEXT:    punpklo p2.h, p0.b
+; CHECK-NEXT:    str p4, [sp, #7, mul vl] // 2-byte Folded Spill
+; CHECK-NEXT:    punpklo p3.h, p2.b
----------------
david-arm wrote:
> david-arm wrote:
> > I don't think this is a problem with your patch, but the spill and fill in this output looks unnecessary. We have enough registers to support this without spilling I think?
> > 
> > Also, something weird seems to be happening with the offset for the spill/fill, i.e. "#7, mul vl". I assume that translates to offset = 7 x vscale x 2? It seems to fit into the stack space we've allocated, but I wonder if this is just pure luck?
> Ah, perhaps we're actually just storing it into the top of the temporary stack space we've allocated, i.e. the top part of a "vscale x 16" byte object.
This happens because p4-p15 are callee-saved, and it needs p4 as a scratch register in this function.

> Ah, perhaps we're actually just storing it into the top of the temporary stack space we've allocated, i.e. the top part of a "vscale x 16" byte object.
Correct. The space it allocates is aligned to <vscale x 16 x i8>, so that's the smallest space that gets allocated, but then it only stores a <vscale x 16 x i1>.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128975