[PATCH] D85220: [CodeGen] Refactor getMemBasePlusOffset & getObjectPtrOffset to accept a TypeSize

Kerry McLaughlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 12 06:41:23 PDT 2020


kmclaughlin added a comment.

In D85220#2211659 <https://reviews.llvm.org/D85220#2211659>, @arichardson wrote:

> This will cause compilation failures in our downstream fork. Is there any reason for replacing the integer overload instead of adding a TypeSize overload?
> I think adding `TypeSize::Fixed(` to all call sites is not ideal since it's overly verbose and also confusing since it's an offset and not a type size.
> It would have also been nice to add a motivation for the change in the commit message rather than just explaining what's changing.

Hi @arichardson, there were a couple of reasons for replacing the integer overload. We wanted to avoid introducing another overloaded function and instead ensure that it is clear to the caller that there is a possibility the offset could be scalable. Additionally, it is possible to cast from `TypeSize` -> `uint64_t`, so removing the integer overload helps to avoid cases in the future where new code accidentally introduces a cast. As more support is added for scalable vectors, there will be many more cases where scalable offsets are required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85220



More information about the llvm-commits mailing list