[PATCH] D26677: [ARM] GlobalISel: Select add i32, i32

[Diana Picus via llvm-commits]

rovka added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/InstructionSelector.cpp:46
 
+    // Predicate operands and optional defs don't need to be constrained.
+    MCOperandInfo OpInfo = I.getDesc().OpInfo[OpI];
----------------
rovka wrote:
> t.p.northover wrote:
> > rovka wrote:
> > > I'm not 100% sure that this is true in the general case. Thoughts?
> > Seems a bit iffy. What was triggering it in ARM?
> For ARM, the predicate register (as added by AddDefaultPred) doesn't have a register class, and the CC register (which is an optional def) isn't found in MRI's VRegInfo (so we get an assert in MRI::getRegClassOrRegBank).
> 
> It's very possible that I'm missing something, but AFAICT the other isel frameworks don't call constrainRegClass on the predicate and optional defs either (at least for ARM). I don't know what is better - rolling a custom version of this function in the ARM backend, or skipping predicates and optional defs here and letting each target handle them. I'm open to other solutions too if you can suggest any :)
@tstellarAMD : I see that some R600 instructions have predicate operands, do you have any thoughts on how it would be best to handle them?


https://reviews.llvm.org/D26677