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

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh 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,
----------------
arsenm wrote:
> What's the point of having 4 copies of 0?
(It was here before, I didn't touch it, just moved it) No point really. I'd be fine with just having a `GICXXPred_Invalid` 

Though, if that's ok, I'd rather fix it in a later diff such as D153757 (which adds more flags here). It makes rebasing less annoying, and none of those patch land unless they all land anyway.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h:329-330
+             "Expected a valid predicate");
+      APFloat Value =
+          State.MIs[InsnID]->getOperand(1).getFPImm()->getValueAPF();
 
----------------
arsenm wrote:
> could this be a const reference? I guess this is a preexisting issue
It was indeed already there but I fixed it anyway, and also fixed the APInt one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153755