[PATCH] D157113: [AArch64] Support more types for ZEXT/SEXT with Global Isel

[Amara Emerson via Phabricator via llvm-commits]

aemerson accepted this revision.
aemerson added a comment.
This revision is now accepted and ready to land.

A few comments but otherwise LGTM, thanks for the new tests.



================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:5951-5952
+    // zext x -> zext(merge(zext(unmerge), zext(unmerge)))
+    Register MidRegister = MRI.createGenericVirtualRegister(MidTy);
+    MIRBuilder.buildInstr(MI.getOpcode(), {MidRegister}, {Src});
+    // Unmerge the vector
----------------
No need for `MidRegister`, this can be simplified to:
```
auto NewExt = MIRBuilder.buildInstr(MI.getOpcode(), {MidTy}, {Src});
...
auto UnmergeSrc = MIRBuilder.buildUnmerge(EltTy, NewExt);
...`


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:5958-5966
+    // ZExt the vectors
+    Register ZExtRes1 =
+        MRI.createGenericVirtualRegister(DstTy.changeElementCount(
+            DstTy.getElementCount().divideCoefficientBy(2)));
+    Register ZExtRes2 =
+        MRI.createGenericVirtualRegister(MRI.getType(ZExtRes1));
+
----------------
Same here, it's less verbose to just create the LLTs and pass those to `buildInstr()` instead of passing Registers.
```
// ZExt the vectors
Register ZExtResTy = DstTy.changeElementCount(DstTy.getElementCount().divideCoefficientBy(2));
auto ZExtRes1 = MIRBuilder.buildInstr(MI.getOpcode(), {ZExtResTy}, {UnmergeSrc.getReg(0)});
auto ZExtRes2 = MIRBuilder.buildInstr(MI.getOpcode(), {ZExtResTy}, {UnmergeSrc.getReg(1)});
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157113