[PATCH] D36534: [aarch64] Support APInt and APFloat in ImmLeaf subclasses and make AArch64 use them.

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: include/llvm/Target/TargetSelectionDAG.td:697
+  let IsAPInt = 1;
+  let FastIselShouldIgnore = 1;
+}
----------------
The comment above says that this the same as ImmLeaf except that Imm is an APInt. But you have also changed the default setting for FastIselShouldIgnore. So if that matters for some target you can't just replace ImmLeaf by IntImmLeaf.

I actually do not get the comments in the summary about the change being NFC for FastISel due to this. You changed lots of  AArch64 definitions to use IntImmLeaf/FPImmLeaf instead of using PatLeaf. But I guess we could see PatLeaf as having FastISelShouldIgnore=0 (because there is no ignore in FastISel for PatLeaf, right?). So to me it looks like you have made changes to how FastISel will handle all those definitions that you have updated to use IntImmLeaf/FPImmLeaf.

I don't know much about FastISel so maybe I misunderstand something. And maybe AArch64 doesn't ever use FastISel. If that is the case then your patch could be seen as NFC, but then I still don't get the point in why it was important to set FastIselShouldIgnore = 1 here.


https://reviews.llvm.org/D36534