[PATCH] D31360: [GlobalISel]: Simple helper functions used across GISel pipeline

[Ahmed Bougacha via Phabricator via llvm-commits]

ab added inline comments.


================
Comment at: include/llvm/CodeGen/GlobalISel/Utils.h:65
+/// Check whether \p Opc is floating point opcode
+bool isFloatingPointOpc(unsigned Opc);
+
----------------
This name is too generic to live in the llvm namespace  (I would expect the function to take Instruction:: opcodes).

More importantly, AArch64RegisterBankInfo.cpp has a 'isPreISelGenericFloatingPointOpcode', which has a different behavior (it ignores itofp, because it interprets 'floating point opcode' as 'all operands are FP values')

IMO that shows that this is too nebulous to be shared.  Keep it next to your use?

Otherwise, maybe we could make this precise enough that we could share it?  For instance, 'isFloatingPointVRegDef' that takes a vreg, checks its definition, and returns true if the def is known to be FP?


================
Comment at: include/llvm/CodeGen/GlobalISel/Utils.h:76
+/// Helper function to get integer value from \p MI at \p Idx
+uint64_t getConstantOperandVal(const MachineInstr &MI, unsigned Idx,
+                               const MachineRegisterInfo &MRI);
----------------
I added a 'getConstantVRegVal' in r298855, but that's only in the selector;  the Optional hopefully lets it replace all three functions (after extracting it from the selector) ?


Repository:
  rL LLVM

https://reviews.llvm.org/D31360