<div dir="ltr">I'll roll this back because of the objections, but I still don't feel "update" is the best verb here. I was looking for a word that matches with other statements doing the same thing using or32{be,le}.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 13, 2016 at 1:43 PM, Davide Italiano <span dir="ltr"><<a href="mailto:davide@freebsd.org" target="_blank">davide@freebsd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Dec 9, 2016 at 12:38 AM, Davide Italiano <<a href="mailto:davide@freebsd.org">davide@freebsd.org</a>> wrote:<br>
> On Thu, Dec 8, 2016 at 9:18 AM, Rui Ueyama via llvm-commits<br>
> <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
>> Author: ruiu<br>
>> Date: Thu Dec  8 11:18:09 2016<br>
>> New Revision: 289072<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=289072&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=289072&view=rev</a><br>
>> Log:<br>
>> Make function names shorter. NFC.<br>
>><br>
>> Modified:<br>
>>     lld/trunk/ELF/Target.cpp<br>
>><br>
>> Modified: lld/trunk/ELF/Target.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Target.cpp?rev=289072&r1=289071&r2=289072&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/ELF/Target.<wbr>cpp?rev=289072&r1=289071&r2=<wbr>289072&view=diff</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- lld/trunk/ELF/Target.cpp (original)<br>
>> +++ lld/trunk/ELF/Target.cpp Thu Dec  8 11:18:09 2016<br>
>> @@ -1320,7 +1320,7 @@ void AArch64TargetInfo::writePlt(<wbr>uint8_t<br>
>>    relocateOne(Buf + 8, R_AARCH64_ADD_ABS_LO12_NC, GotEntryAddr);<br>
>>  }<br>
>><br>
>> -static void updateAArch64Addr(uint8_t *L, uint64_t Imm) {<br>
>> +static void write32addr(uint8_t *L, uint64_t Imm) {<br>
>>    uint32_t ImmLo = (Imm & 0x3) << 29;<br>
>>    uint32_t ImmHi = (Imm & 0x1FFFFC) << 3;<br>
>>    uint64_t Mask = (0x3 << 29) | (0x1FFFFC << 3);<br>
>> @@ -1334,8 +1334,8 @@ static uint64_t getBits(uint64_t Val, in<br>
>>    return (Val >> Start) & Mask;<br>
>>  }<br>
>><br>
>> -// Update the immediate field in a ldr, str, and add instruction.<br>
>> -static inline void updateAArch64LdStrAdd(uint8_t *L, uint64_t Imm) {<br>
>> +// Update the immediate field in a AARCH64 ldr, str, and add instruction.<br>
><br>
> This is far less than ideal. Now I need to look at the code to<br>
> understand what the function does, which wasn't true before.<br>
> Saving few characters is not worth the gain, IMHO. This is not a<br>
> StringRef function that's used in hundreds of files in LLVM, it's just<br>
> a static function used in few places in this file. It's fairly<br>
> unlikely there are gonna be more uses of it, so I don't see the<br>
> rationale behind this change. Also, I think the correct spelling is<br>
> AArch64, and not AARCH64 (but I would confirm with Renato or some of<br>
> the other ARM folks). Ditto for the other function.<br>
><br>
<br>
</div></div>ping.<br>
<div class="HOEnZb"><div class="h5"><br>
--<br>
Davide<br>
<br>
"There are no solved problems; there are only problems that are more<br>
or less solved" -- Henri Poincare<br>
</div></div></blockquote></div><br></div>