[PATCH] D77939: [MC] Use subclass data for MCExpr to reduce memory usage

[Jinsong Ji via Phabricator via llvm-commits]

jsji added inline comments.


================
Comment at: include/llvm/MC/MCExpr.h:46
 private:
+  enum { NumSubclassDataBits = 24 };
+
----------------
nikic wrote:
> jsji wrote:
> > Why we choose enum here? 
> > Can we set it to const calculated from `sizeof(unsigned int)`?
> > Why we choose enum here? 
> 
> I'm actually not sure... I followed what I saw other similar code doing. My assumption was that some compilers do not allow using static const in bitfield widths (as they aren't "real" constants), but after some googling around I didn't find any evidence of that, so I'll switch this to use `static const` and go back to the enum if it breaks any bots.
> 
> > Can we set it to const calculated from sizeof(unsigned int)?
> 
> Something like `CHAR_BIT * (sizeof(unsigned int) - sizeof(uint8_t))`? I'm not sure this is better, because it makes it non-obvious how much space is actually free to use.
>Something like CHAR_BIT * (sizeof(unsigned int) - sizeof(uint8_t))? I'm not sure this is better, because it makes it non-obvious how much space is actually free to use.

OK, make sense too. Maybe add a comments to 24 == CHAR_BIT * (sizeof(unsigned int) - sizeof(ExprKind))?


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

https://reviews.llvm.org/D77939