[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