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

[Ahmed Bougacha via Phabricator via llvm-commits]

ab accepted this revision.
ab added a comment.
This revision is now accepted and ready to land.

LGTM without the pattern changes.



================
Comment at: include/llvm/CodeGen/GlobalISel/InstructionSelector.h:83
 
+  bool constrainOperandRegToRegClass(MachineInstr &I, unsigned OpIdx,
+                                     const TargetRegisterClass &RC,
----------------
Comment?  Or, alternatively, inline the one-line body in the emitted code?


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.td:5588-5592
+          (COPY_TO_REGCLASS FPR32:$Xn, GPR32all)>;
 def : Pat<(f64 (bitconvert (i64 GPR64:$Xn))),
           (COPY_TO_REGCLASS GPR64:$Xn, FPR64)>;
 def : Pat<(i64 (bitconvert (f64 FPR64:$Xn))),
+          (COPY_TO_REGCLASS FPR64:$Xn, GPR64all)>;
----------------
I don't think this is correct: FMOV really does take GPR32/GPR64, not GPR32all/GPR64all, and that's what copyPhysReg looks for.

And the c++ selector actually also looks wrong.  I'm guessing it happens to work because we never allocate SP, so anything we allocate out of GPR32all is in GPR32 as well anyway.


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:1046
+class ConstrainOperandsToDefinitionAction : public MatchAction {
+private:
+  std::string Name;
----------------
Unnecessary 'private', here and below.


https://reviews.llvm.org/D33590