[PATCH] D87285: AMDGPU/GlobalISelemitter Support for predicate code that uses operands

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h:518
+  testMIPredicate_MI(unsigned, const MachineInstr &,
+                     const SmallVectorImpl<MachineOperand *> &Operands) const {
     llvm_unreachable(
----------------
Can't this be an ArrayRef<MachineOperand *> ? (Or a pointer to const std::array ?)


================
Comment at: llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp:33
+    : Renderers(MaxRenderers), MIs() {
+  RecordedOperands.resize(3);
+}
----------------
Petar.Avramovic wrote:
> arsenm wrote:
> > Why arbitrarily resize this here?
> Operands are inserted directly into specific index in RecordedOperands, see GIM_RecordNamedOperand.
> Magic number 3 is the largest number of operands that any of the patterns with  'let PredicateCodeUsesOperands = 1' has.
> I wanted to avoid having to keep additional maps between operand names and indices in final operand list that will be sent as argument to c++ predicate code' (like sdag). Unlike sdag, globalisel emitter first visits the predicate and we can figure out indices right away, sdag visits all operands first and predicate is visited last. 
If you really want a fixed size array, wouldn't std::array be better?


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

https://reviews.llvm.org/D87285