[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