[PATCH] D36569: [globalisel][tablegen] Add support for fpimm and import of APInt/APFloat based ImmLeaf.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

Thanks for the reviews



================
Comment at: lib/Target/AArch64/AArch64InstructionSelector.cpp:709
+
+      // 0.0 is covered by tablegen.
+      if (I.getOperand(1).getFPImm()->getValueAPF().isExactlyValue(0.0))
----------------
qcolombet wrote:
> I didn't get that comment.
> Could you elaborate?
This patch causes the rules involving fpimm0 to be imported so we no longer need to handle that case in the C++. It's not harmful to leave it in, but doing so means we can't tell if the test case passes because tablegen works or because the C++ caught it. The comment is intended to explain why we choose not to handle 0.0 in the C++ code even though it would be valid.


================
Comment at: utils/TableGen/CodeGenDAGPatterns.h:485
+  /// Get the opcode used to check this predicate.
+  std::string getGlobalISelMatchOpcode() const;
+
----------------
qcolombet wrote:
> Could we move these in GlobalISelEmitter?
> Something like GlobalISelEmitter::getPredicateOpcode(TreePredicateFn)?
Ok


================
Comment at: utils/TableGen/CodeGenDAGPatterns.h:495
+  /// usable as part of an identifier.
+  std::string getImmTypeIdentifier() const;
 };
----------------
qcolombet wrote:
> Ditto.
Ok


https://reviews.llvm.org/D36569