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

[Paul C. Anagnostopoulos via Phabricator via llvm-commits]

Paul-C-Anagnostopoulos added a comment.

I've been doing it for documentation changes, commenting, and certain other relatively trivial changes, That was suggested by @lattner, not decided unilaterally.

You are right, though, that this is not trivial, so I withdraw my auto-LGTM comment.



================
Comment at: llvm/include/llvm/MC/MCInstrDesc.h:43
+
+#define MCOI_EARLY_CLOBBER(bit) \
+  ((1 << MCOI::EARLY_CLOBBER) | ((bit) << (4 + MCOI::EARLY_CLOBBER * 4)))
----------------
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.


================
Comment at: llvm/utils/TableGen/InstrInfoEmitter.cpp:186
+        Res += "MCOI_EARLY_CLOBBER(1)";
+////        Res += "(1 << MCOI::EARLY_CLOBBER)";
       else {
----------------
craig.topper wrote:
> Drop commented out code?
Oh yes, I will clean out the old code.


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

https://reviews.llvm.org/D93326