[PATCH] D46746: [RISCV] Fix builtin fixup sizes

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 16 11:01:47 PDT 2018


asb added a comment.

In https://reviews.llvm.org/D46746#1098532, @asb wrote:

> Thanks Simon. I can see that truncation of FK_Data_8  is a correctness issue and addressing that in getSize seems most sensible.
>
> I wonder if changing getSize is the correct place to be fixing `FK_DATA_1` and `FK_DATA_2` though. After all, all the target-specific fixups are fully dependent on fixup overflow not taking place.  If (for instance) it's possible for an `FK_DATA_1` fixup to give a value that's greater than 8 bits, maybe the bug is in adjustFixupValue?
>
> It looks like SystemZMCAsmBackend::applyFixup demonstrates an even better approach: don't rely on a helper function at all and just use getFixupKindInfo to get the size (in bits) of the fixup. What do you think about adapting that sort of logic instead?


I looked at this in more detail, and of course we're already using getFixupKindInfo to find the size in bits of the fixup. I think https://reviews.llvm.org/D46965 is a reasonable alternative approach - could you please check I'm not missing anything obvious.


Repository:
  rL LLVM

https://reviews.llvm.org/D46746





More information about the llvm-commits mailing list