[PATCH] D95326: [GlobalISel] Implement widenScalar for carry-in add/sub

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:1769
+  Optional<Register> CarryIn = None;
+  switch (MI.getOpcode()) {
+  case TargetOpcode::G_SADDO:
----------------
Can you add an unreachable case to make sure nobody passes an unexpected instruction here?


================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/legalize-sadde.mir:5
+---
+name:            test_scalar_sadde_small
+body:             |
----------------
Can these testcases have slightly more descriptive names?


================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/legalize-sadde.mir:32
+    %4:_(s8) = G_TRUNC %1(s64)
+    %5:_(s1) = G_TRUNC %2(s64)
+    %6:_(s8), %7:_(s1) = G_SADDE %3, %4, %5
----------------
Nit: we can give virtual registers names, and it usually makes it way easier to read + understand testcases (IMO).

Since the new testcases here are all pretty similar, do you think you could add names to the virtual registers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95326