[PATCH] D46695: [RFC] [Patch 1/3] Add a new class of predicates for variant scheduling classes.

[Renato Golin via Phabricator via llvm-commits]

rengolin added inline comments.


================
Comment at: include/llvm/Target/TargetInstrPredicate.td:110
+// operand at position `Index` is a register operand.
+class CheckRegOperandValue<int Index, Register R>
+    : MCOperandPredicate<Index> {
----------------
RKSimon wrote:
> rengolin wrote:
> > andreadb wrote:
> > > RKSimon wrote:
> > > > The use of 'Value' makes is it sound like we need the register to contain a certain value, not that it must be a certain register.
> > > I am okay with bikeshedding names if people don't like them.
> > > 
> > > What if I change `CheckRegOperand` into `CheckOperandIsRegister`, and then rename `CheckRegOperandValue` to `CheckRegOperand`?
> > This would make git archaeology confusing... :) 
> @rengolin Do you mean if the names have to change in the future after the patch is committed? A big concern is tablegen's lack of scoping might cause clashes in the future.
No, the proposal seem to be A -> B, C -> A; which will make confusing to compare patches on the two sides of the change regarding `CheckRegOperand` and what it means, but that's probably a minor concern.

Anyhow, I'm not trying to chime in on a bikeshedding argument. :)


https://reviews.llvm.org/D46695