[PATCH] D33590: [globalisel][tablegen] Add support for COPY_TO_REGCLASS.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

The remaining issue I mentioned in the update is this one.

> - Double check why tablegen gives us gpr32 instead of gpr32all

This happens because the SelectionDAG rules specify gpr32. For example:

  // (bitconvert:i32 FPR32:f32:$Xn) => (COPY_TO_REGCLASS:i32 FPR32:f32:$Xn, GPR32:i32)

We can probably change the SelectionDAG rules. I'll give this a try



================
Comment at: test/CodeGen/AArch64/GlobalISel/select-bitcast.mir:98
 # CHECK-NEXT:  - { id: 0, class: fpr32 }
-# CHECK-NEXT:  - { id: 1, class: gpr32all }
+# CHECK-NEXT:  - { id: 1, class: gpr32 }
 registers:
----------------
qcolombet wrote:
> That change is strange. How does tablegen come up with this constraint?
> I would have expected that we get the less constrained regclass (gpr32all in that case)
It's explicitly stated to be GPR32 in the SelectionDAG rule. Here's the pattern comment from the imported rules:
// (bitconvert:i32 FPR32:f32:$Xn)  =>  (COPY_TO_REGCLASS:i32 FPR32:f32:$Xn, GPR32:i32)



================
Comment at: test/CodeGen/AArch64/GlobalISel/select-bitcast.mir:197
 # CHECK-NEXT:  - { id: 0, class: fpr64 }
-# CHECK-NEXT:  - { id: 1, class: gpr64all }
+# CHECK-NEXT:  - { id: 1, class: gpr64 }
 registers:
----------------
qcolombet wrote:
> Ditto
This one has the same cause:
// (bitconvert:i64 FPR64:f64:$Xn)  =>  (COPY_TO_REGCLASS:i64 FPR64:f64:$Xn, GPR64:i32)



================
Comment at: test/TableGen/GlobalISelEmitter.td:476
+// CHECK-NEXT:        ((/* dst */ (MRI.getType(MI0.getOperand(0).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT:         ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(0).getReg(), MRI, TRI))))) &&
+// CHECK-NEXT:        ((/* src1 */ (MRI.getType(MI0.getOperand(1).getReg()) == (LLT::scalar(32))) &&
----------------
qcolombet wrote:
> Remark: Long term we should be able to use the right regbank directly.
I agree. I manually experimented with this a bit while fixing the compile-time regression. To do it properly I'll need to refactor some of the RegisterBankEmitter to be able to re-use this information from other tablegen emitters.


================
Comment at: test/TableGen/GlobalISelEmitter.td:488
+
+// The -6 is just to distinguish it from the other 'not' cases.
+def : Pat<(i32 (bitconvert FPR32:$src1)),
----------------
qcolombet wrote:
> I didn't get that BTW
This is a bad copy/paste. It's referring to something that only matters to the xor rules.


https://reviews.llvm.org/D33590