[PATCH] D74338: [RFC][TableGen/RISCV] Support combining AssemblerPredicates with ORs

[Lewis Revill via Phabricator via llvm-commits]

lewis-revill added a comment.

I can't see any issues with the logic of this implementation. It doesn't lend itself well to adapting it in the future if we want to be able to combine the operators, but I'd expect that would require a more disruptive rewrite so I'm not //too// concerned about that right now. See comments though.



================
Comment at: llvm/include/llvm/MC/MCInstPrinter.h:135
+    K_OrFeature,  // Match only if one of a set of features is enabled.
+    K_OrNegFeature,  // Match only if one of a set of features is disabled.
+    K_EndOrFeatures, // Note end of list of K_Or(Neg)?Features.
----------------
Nitpick: Realign comments in this enum


================
Comment at: llvm/lib/MC/MCInstPrinter.cpp:149
+    bool OrPredicateResult = false;
     if (llvm::all_of(Conds, [&](const AliasPatternCond &C) {
+          return matchAliasCondition(*MI, STI, MRI, OpIdx, M, C,
----------------
I don't know this code well enough to know if this is feasible but could this work by instead checking at this point if the pattern is an 'or' pattern, and selecting using 'any_of' if that's the case? We could then avoid adding the K_Or conditions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74338