[PATCH] D44304: [MIPS GlobalISel] Select add i32, i32

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added reviewers: aditya_nandakumar, qcolombet.
dsanders added subscribers: aditya_nandakumar, qcolombet.
dsanders added a comment.

Sorry for taking a while to get to this. It's been a busy couple weeks

The ISel and RegisterBank parts look good to me with a couple convention nits fixed.

@aditya_nandakumar: You're more familiar with GlobalISel's calling convention code than I am. Do you have any comments on that part?
@qcolombet: Do you have any comments on the regbankselect part?



================
Comment at: lib/Target/Mips/MipsInstructionSelector.cpp:22
 
+#define DEBUG_TYPE "Mips-isel"
+
----------------
I'd recommend making the 'M' lower case here. Very few DEBUG_TYPE's have uppercase letters and there's no error for picking a value that doesn't exist


================
Comment at: lib/Target/Mips/MipsRegisterBanks.td:13
+
+def GPRRegBank : RegisterBank<"GPRb", [GPR32]>;
----------------
It's not a requirement but by convention the string should match the def (e.g. `def XRegBank : RegisterBank<"X", ...>`). ARM doesn't do that because it has a register class by the same name and banks/classes share a namespace in MIR.


https://reviews.llvm.org/D44304