[PATCH] D153791: [MachineInst] Bump NumOperands back up to 24bits

Jon Roelofs via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 26 13:49:37 PDT 2023


jroelofs added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:137
+    uint8_t AsmPrinterFlags : LLVM_MID_ASMPRINTERFLAGS_BITS;
+  })
+  Detail = {0, 0, 0};
----------------
nikic wrote:
> jroelofs wrote:
> > nikic wrote:
> > > Why do we need the packed struct wrapper here? Doesn't defining these bitfields directly in the class work?
> > The two 24-bit fields need to be part of a packed struct, otherwise we get some padding [1]. We can either do that around the whole struct, or just these 56 bits. I don't have a strong opinion either way.
> > 
> > 1: https://clang.godbolt.org/z/4Kxs4fKc7
> Can you avoid the need for that by reordering the fields to be 24 8 24 8? E.g. Flags, AsmPrinterFlags, NumOperands, CapOperands.
Good eye! Yeah, that works: https://clang.godbolt.org/z/q1onMv94e


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

https://reviews.llvm.org/D153791



More information about the llvm-commits mailing list