[PATCH] D29478: [GlobalISel] Generate selector with predicates; use it for FP binops.
Kristof Beyls via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 3 04:35:42 PST 2017
kristof.beyls added inline comments.
Comment at: utils/TableGen/GlobalISelEmitter.cpp:315-317
+ Optional<std::string> Predicate;
I've gotten the feedback that most developers don't really understand how the auto-generated "magic" works for DAGISel.
So far, I think the current design is pretty easy to understand in comparison. I've tried being picky in this area before about things like naming to keep the code as-easy-to-understand as possible.
Maybe the 3 variables here could use a short comment in that respect?
Maybe something like
/// optional code string that indicates when this rule is legal
/// list of matchers that all need to evaluate true for the current IR node to match pattern P.
/// re-writing actions that need to be taken when the current IR node matches pattern P and Predicate is true.
Please note that I haven't tried hard to make the comment fully factual correct - I'm hoping Daniel or Ahmed will be able to quickly make those comments factually correct.
Comment at: utils/TableGen/GlobalISelEmitter.cpp:349-369
OS << " // Src: " << *P.getSrcPattern() << "\n"
<< " // Dst: " << *P.getDstPattern() << "\n";
+ OS << " if (";
+ // Emit the rule-level predicates, if any.
+ if (Predicate)
I think it's slightly easier to read if the assert would go before any of the code-emitting code.
It makes it slightly easier to read what code gets generated.
More information about the llvm-commits