[PATCH] D39823: GlobalISel: Enable the legalization of G_MERGE_VALUES and G_UNMERGE_VALUES

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerInfo.cpp:194-201
+    unsigned Op = MI.getOperand(i).getReg();
+    // G_UNMERGE_VALUES has variable number of operands, but there is only
+    // one source type and one destination type as all destinations must be the
+    // same type. So, get the last operand if TypeIdx == 1.
+    if (MI.getOpcode() == TargetOpcode::G_UNMERGE_VALUES && TypeIdx == 1) {
+      Op = MI.getOperand(MI.getNumOperands() - 1).getReg();
+    }
----------------
To make this easier to read, I think it may make sense to factor this out into a separate function, e.g. with a name like "getTyFromTypeIdx(MI, TypeIdx)". I'm not sure on the name, but doing it this way would make this code a lot easier to understand, I think.


================
Comment at: lib/Target/AArch64/AArch64LegalizerInfo.cpp:359
+  for (const auto &Ty :
+       {s32, s64, s96, s128, s192, v2s32, v3s32, v4s32, v2s64, v4s64, v6s64}) {
+    setAction({G_MERGE_VALUES, Ty}, Legal);
----------------
I haven't thought this through at all, but I'm a bit surprised v8s64 also isn't legal if v6s64 is?
Similarly for S256 if s192 is legal?


https://reviews.llvm.org/D39823