[PATCH] D82962: [Alignment][NFC] Use Align for BPFAbstractMemberAccess::RecordAlignment

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet marked an inline comment as done.
gchatelet added inline comments.


================
Comment at: llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp:120
     uint32_t AccessIndex;
-    uint32_t RecordAlignment;
+    Align RecordAlignment;
     MDNode *Metadata;
----------------
courbet wrote:
> I cannot convince myself that this is an NFC.  Aggregate initialization will initialize `RecordAlignment` to `0`, and there are some cases when `RecordAlignment` is not set by code (e.g. `BPFAbstractMemberAccess::IsPreserveDIAccessIndexCall`). 
> 
> On the other hand, the only usage is
> ```
> uint32_t MemberBitSize = MemberTy->getSizeInBits();
>   uint32_t MemberBitOffset = MemberTy->getOffsetInBits();
>   uint32_t AlignBits = RecordAlignment * 8;
>   if (RecordAlignment > 8 || MemberBitSize > AlignBits)
>     report_fatal_error("Unsupported field expression for llvm.bpf.preserve.field.info, "
>                        "requiring too big alignment");
> ```
> 
> So it looks like `0` would always lead to a fatal error anyway, so non-zero is really not intended.
I find this puzzling because the error specifically mentions "llvm.bpf.preserve.field.info" and if you look at `IsPreserveDIAccessIndexCall`, the only case where `RecordAlignment` can be `0` is when `GV->getName().startswith("llvm.bpf.preserve.field.info")`. On all other cases `RecordAlignment>0` and `GV->getName()` would start with one of:
 - "llvm.preserve.array.access.index"
 - "llvm.preserve.union.access.index"
 - "llvm.preserve.struct.access.index"

So it doesn't make sense to me.

As you said, `0` would lead to a fatal error so I take this as `RecordAlignment!=0`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82962