Add the minimal support necessary to lower a function that returns the sum of two i32 values.
Support argument/return lowering of i32 values through registers only.
Add tablegen for regbankselect and instructionselect.
Details
Diff Detail
Event Timeline
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?
lib/Target/Mips/MipsInstructionSelector.cpp | ||
---|---|---|
22 | 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 | |
lib/Target/Mips/MipsRegisterBanks.td | ||
14 | 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. |
@aditya_nandakumar: You're more familiar with GlobalISel's calling convention code than I am. Do you have any comments on that part?
From a quick glance, it looks good to me..
Addressed review comments, update CMakeLists.txt according to changes from rL329181.
Also fix comments about where pointers will be handled.
The LLVM-IR in the MIR test files doesn't match the MIR below, but I haven't checked if that makes a difference.
test/CodeGen/Mips/GlobalISel/instruction-select/add.mir | ||
---|---|---|
6 | This IR doesn't match the MIR below. This function takes two parameters, but doesn't return anything or perform any computation. The MIR below does the trivial add two parameters and return the result. |
No, it doesn't make a difference, except that this way the test is shorter. The alternative is to put the whole ll function like one in add.ll.
Ok. The translation is covered by the irtranslator tests, so we should catch any unexpected changes.
LGTM.
test/CodeGen/Mips/GlobalISel/instruction-select/add.mir | ||
---|---|---|
6 | That's actually fine. MIR mostly only cares about the prototype and having declarations for memory objects. You can often delete everything from the '--- |' to the '...' | |
14–17 | ... which reminds me. You can probably delete this 'registers' section too so long as the constraints are defined inline in the body which they appear to be |
test/CodeGen/Mips/GlobalISel/instruction-select/add.mir | ||
---|---|---|
14–17 | I have deleted it in r329888. Thanks. |
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