[PATCH] D88614: [AArch64][GlobalISel] NFC: Refactor G_FCMP selection code

[Amara Emerson via Phabricator via llvm-commits]

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

A few minor comments.



================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:177
+  /// Emit a floating point comparison between \p LHS and \p RHS, which
+  /// checks \p Pred.
+  MachineInstr *emitFPCompare(Register LHS, Register RHS,
----------------
No Pred in args.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:251
+  /// Emit a CSet for a FP compare.
+  MachineInstr *emitCSetForFCmp(Register Dst, CmpInst::Predicate Pred,
+                                MachineIRBuilder &MIRBuilder) const;
----------------
Seeing as we seem to only ever need the 32 bit variant. Can document DstReg to be a 32 bit scalar register. And in the bodies assert that its a 32 bit scalar type rather than return nullptr.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:3937
+    return nullptr;
+  Register ZeroReg = AArch64::WZR;
+
----------------
This can be const.


================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/select-fcmp.mir:79
+    %0:fpr(s64) = COPY $d0
+    %1:fpr(s64) = COPY $d1
+    %2:fpr(s64) = G_FCONSTANT double 1.000000e+00
----------------
Nit: %1 seems to be unused in these two tests (and others in the file but no bother).


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

https://reviews.llvm.org/D88614