[PATCH] D78092: [GlobalISel] add legalization for G_FREEZE

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:843-846
+    SmallVector<Register, 2> SrcRegs, LeftoverRegs, DstRegs, LeftoverDstRegs;
+    if (!extractParts(SrcReg, SrcTy, NarrowTy, LeftoverTy, SrcRegs,
+                      LeftoverRegs))
+      return UnableToLegalize;
----------------
I've been working on eliminating these somewhat awkward functions that end up producing G_INSERT/G_EXTRACT in odd breakdowns instead of creating intermediate merges to the requested type. fewerElementsVectorBasic should be pretty close to what this should do? (Although maybe pad with undef is the wrong choice here)


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:856-857
+      const Register PartDstReg = MRI.createGenericVirtualRegister(LeftoverTy);
+      MIRBuilder.buildInstr(TargetOpcode::G_FREEZE, {PartDstReg},
+                            {LeftoverReg});
+      LeftoverDstRegs.emplace_back(PartDstReg);
----------------
Should have a dedicated buildFreeze


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:1772
+    Observer.changingInstr(MI);
+    widenScalarSrc(MI, WideTy, 1, TargetOpcode::G_ANYEXT);
+    widenScalarDst(MI, WideTy);
----------------
Is this what the DAG does? I would be slightly surprised that undef high bits would be OK


================
Comment at: llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp:2970
+// Test more elements of G_FREEZE
+TEST_F(AArch64GISelMITest, MoreElementsFreeze) {
+  setUp();
----------------
If you want some tests that are less inconvenient than unit tests, for AMDGPU this should have the same legalization action as G_IMPLICIT_DEF (and probably true for all targets)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78092