[PATCH] D125485: [ArgPromotion] Unify byval promotion with non-byval

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D125485#3515516 <https://reviews.llvm.org/D125485#3515516>, @psamolysov wrote:

> @nikic If you have time, could you make it clear why the `OffsetAndArgPart` alias is defined as `std::pair<int64_t, ArgPart>`, so why `int64_t` is used for Offset, not `uint64_t`? All the LLVM's API for work with offsets, `APInt` (is created from an `uint64_t` value), `StructLayout::getElementOffset` (returns `uint64_t`), etc use unsigned values. Do we really use negative offsets and if not, can the offset part of the pair be replaced with `uint64_t`? Thank you.

This is an int64_t, because GEP offsets in LLVM are signed. This is only supported "because we can" though, so if it makes things complicated it could be dropped (this would require bailing out of the transform for negative offsets, not just changing the type). Note that for byval, an access at negative offset is UB, so we're free to transform it even if it makes no sense in that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125485