[PATCH] D153755: [GlobalISel] Generalize `InstructionSelector` Match Tables

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h:77-80
+  GICXXPred_I64_Invalid = 0,
+  GICXXPred_APInt_Invalid = 0,
+  GICXXPred_APFloat_Invalid = 0,
+  GICXXPred_MI_Invalid = 0,
----------------
What's the point of having 4 copies of 0?


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h:448
 
-/// Provides the logic to select generic machine instructions.
-class InstructionSelector {
+/// Provides the logic to execute GlobalISel match tables, why are used by the
+/// instruction selector and instruction combiners as their engine to match and
----------------
wrong word why


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h:329-330
+             "Expected a valid predicate");
+      APFloat Value =
+          State.MIs[InsnID]->getOperand(1).getFPImm()->getValueAPF();
 
----------------
could this be a const reference? I guess this is a preexisting issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153755