[PATCH] D36534: [aarch64] Support APInt and APFloat in ImmLeaf subclasses and make AArch64 use them.
Diana Picus via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 30 07:44:33 PDT 2017
rovka added a comment.
I can see why you need to change the patterns to be more general, but is the distinction between APInt, APFloat and 64-bit values really relevant? I looked a bit at the following patch which uses this for GlobalISel and I'm wondering if we could just get away with checking for each immediate a combination of isCImm, isFPImm and getBitWidth (so we wouldn't need to introduce different states for different kinds of immediates).
I'm sure I'm missing some details, so what's the catch? :)
================
Comment at: utils/TableGen/CodeGenDAGPatterns.cpp:787
+ return "const APInt &";
+ else if (immCodeUsesAPFloat())
+ return "const APFloat &";
----------------
Redundant else.
================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:2555
+ << "enum {\n";
+ StringRef EnumeratorSeparator = " = GIPFP_Invalid,\n";
+ for (const auto *Record : RK.getAllDerivedDefinitions("PatFrag")) {
----------------
Shouldn't this be +1?
================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:2568
+ if (!Record->getValueAsString("ImmediateCode").empty())
+ if (Filter(Record))
+ OS << " static bool Predicate_" << Record->getName()
----------------
Could you get rid of this repetition? If we don't expect many of these, we could probably use a copy_if into another container, but if you think that might affect performance too much you can probably just extract a ForEachFilteredImm lambda/helper. I can think of other variations but I'll let you choose whatever you think is best.
https://reviews.llvm.org/D36534
More information about the llvm-commits
mailing list