[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

Oliver Stannard (Linaro) via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 27 03:31:36 PST 2020


ostannard added a comment.

> It will require changing all possible initializations, with a sensible value.

Where are you seeing multiple initializations? It looks like all of the logic for struct layout happens in `CGRecordLowering::lower`, we might need to add a pass there to calculate the valid access widths for bitfields.

> And either way, codegen will also need to change, to define the address, it must know if it is volatile or not.

The difference is that codegen only needs to change to check if the access is volatile, and select the correct offset and size already calculated during struct layout.

> perhaps we can leave it as a TODO to move it there?

I'm less concerned about the performance impact of this, and more about making this code harder to understand by spreading the logic across multiple different phases of compilation. This has been broken for years, I'd rather we fix it properly, than add a TODO which will never get done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72932





More information about the cfe-commits mailing list