[PATCH] D93326: [MCInstrDesc] [TableGen] Reduce size of MCOperandInfo instances

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/include/llvm/MC/MCInstrDesc.h:43
+
+#define MCOI_EARLY_CLOBBER(bit) \
+  ((1 << MCOI::EARLY_CLOBBER) | ((bit) << (4 + MCOI::EARLY_CLOBBER * 4)))
----------------
Paul-C-Anagnostopoulos wrote:
> craig.topper wrote:
> > Paul-C-Anagnostopoulos wrote:
> > > craig.topper wrote:
> > > > What is "bit" here? There's no data associated with EARLY_CLOBBER. The original code just did (1 << MCOI::EARLY_CLOBBER) to set the flag, there was nothing in the upper bits.
> > > All the calls to get the EARLY_CLOBBER constraint only check for -1, the value returned when the constraint is not present. So it's a boolean, but based on its presence rather than its value. That seemed odd to me, so my plan is to change the calls to check for =1 rather than !=-1.
> > > 
> > > I could be convinced that presence/absence is a perfectly good boolean.
> > What about this?
> I was going to wait to make the =1 change until after this is pushed. Do you think I should do it now?
I think I must have wrote my comment at the same time you were writing yours.

I think this patch should keep just (1 << MCOI::EARLY_CLOBBER)  and not have the bit argument. That would be consistent with what tablegen is doing before this patch.


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

https://reviews.llvm.org/D93326