[PATCH] D68426: [gicombiner] Hoist pure C++ combine into the tablegen definition

[Daniel Sanders via Phabricator via llvm-commits]

dsanders marked 2 inline comments as done.
dsanders added inline comments.


================
Comment at: llvm/utils/TableGen/GICombinerEmitter.cpp:269
+
+    OS << Indent << "  if (1\n";
+
----------------
volkan wrote:
> Rules must have a matcher, right? If so, why do we need `1 &&` here? 
Yes but they don't necessarily use the trap-door into the C++ block (eventually none will and I'll remove it). At this point in the patch series, that trap-door is the only thing the matcher can use so they all use it but in subsequent patches there will be enough features to move the code into DAG matchers and/or custom predicates. Even later, predicates that don't get tested as part of the decision tree will be resolved in this if-statement.

For why the `1 &&` is in the output, it's just to keep the code simple. Without it, I'd have to special case the case where there's no checks but with it I can just have each check emit `&& foo()`


================
Comment at: llvm/utils/TableGen/GICombinerEmitter.cpp:282
+         << Indent << "      "
+         << CodeExpander(Rule->getCode()->getValue(), Expansions,
+                         Rule->getCode()->getLoc(), ShowExpansions)
----------------
volkan wrote:
> Don't we need to get the Matcher here as we do that for Applyer below?
At the moment, the matcher can only contain a code block and this is essentially a trap-door that allows me to fix up the generated code and keep things working while the features are going in. I could rename getCode() to getMatcherFixupCode() if that helps.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68426/new/

https://reviews.llvm.org/D68426