[PATCH] D66180: [GlobalISel][CallLowering] Add support for splitting types according to calling conventions

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CallLowering.cpp:198-200
+        if (CurVT.isVector()) {
+          return false;
+        }
----------------
obligatory braces nit


================
Comment at: llvm/lib/CodeGen/GlobalISel/CallLowering.cpp:206
+
+      if (Handler.isIncomingArgumentHandler()) {
+        if (NumParts == 1) {
----------------
I think a high-level explanation of what we're going to do when we have an incoming vs outgoing handler would be useful.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CallLowering.cpp:220
+          LLT NewLLT = getLLTForMVT(NewVT);
+          for (unsigned Part = 0; Part < NumParts; ++Part) {
+            Register Reg =
----------------
Can we have a comment here explaining what's going on?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CallLowering.cpp:225-227
+            if (Handler.assignArg(i, NewVT, NewVT, CCValAssign::Full, Args[i],
+                                  Args[i].Flags[Part], CCInfo))
+              return false;
----------------
I think that this could use a comment too.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CallLowering.cpp:232
+        // Handling an outgoing arg that might need to be split.
+        if (NumParts < 2) {
+          // Don't know how to deal with this type combination.
----------------
paquette wrote:
> braces
Why less than 2?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CallLowering.cpp:232-235
+        if (NumParts < 2) {
+          // Don't know how to deal with this type combination.
+          return false;
+        }
----------------
braces


================
Comment at: llvm/lib/CodeGen/GlobalISel/CallLowering.cpp:285
+          // Expected to be multiple regs for a single incoming arg.
+          if (Args[i].Regs.size() < 2)
+            return false;
----------------
Pull `Args[I].Regs.size()` into a variable with a meaningful name?


================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/arm64-callingconv.ll:1
-; RUN: llc -O0 -stop-after=irtranslator -global-isel -verify-machineinstrs %s -o - 2>&1 | FileCheck %s
+; RUN: llc -O0 -stop-after=irtranslator -global-isel -global-isel-abort=1 -verify-machineinstrs %s -o - 2>&1 | FileCheck %s
 
----------------
Why are we falling back now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66180