[PATCH] D14922: [ELF/AArch64] Factor out overflow check into a separate function.
Igor Kudrin via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 23 21:28:31 PST 2015
On 24.11.2015 01:45, Rui Ueyama wrote:
> On Mon, Nov 23, 2015 at 11:40 AM, Rafael EspĂndola
> <rafael.espindola at gmail.com <mailto:rafael.espindola at gmail.com>> wrote:
>
> On 23 November 2015 at 13:04, Rui Ueyama via llvm-commits
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>>
> wrote:
> > ruiu added a comment.
> >
> > I'm not in favor of this change, and I wouldn't do that. The
> previous code was super clear that it errors if isInt check fails.
> Now you have to take a look at the definition of
> checkAArch64OutOfRange function. You saved one line per a function
> call, but it doesn't seem like a good tradeoff from code
> readability point of view.
>
> How about something like
>
> checkIsInt<N>(Val, Msg);
>
> It should be almost as compact as this change and easy to read.
>
>
> That may make sense. I'm not sure if I'll like that than the original
> code, but it's worth a try.
>
The main motivation was to simplify the code that generates error
messages, like this:
if (Type == R_AARCH64_JUMP26)
error("Relocation R_AARCH64_JUMP26 out of range");
error("Relocation R_AARCH64_CALL26 out of range");
Maybe I got a bit carried away. I'll prepare a new patch today.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151124/d7f28008/attachment.html>
More information about the llvm-commits
mailing list