[lld] r289072 - Make function names shorter. NFC.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 13 14:44:30 PST 2016


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}.

On Tue, Dec 13, 2016 at 1:43 PM, Davide Italiano <davide at freebsd.org> wrote:

> On Fri, Dec 9, 2016 at 12:38 AM, Davide Italiano <davide at freebsd.org>
> wrote:
> > On Thu, Dec 8, 2016 at 9:18 AM, Rui Ueyama via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >> Author: ruiu
> >> Date: Thu Dec  8 11:18:09 2016
> >> New Revision: 289072
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=289072&view=rev
> >> Log:
> >> Make function names shorter. NFC.
> >>
> >> Modified:
> >>     lld/trunk/ELF/Target.cpp
> >>
> >> Modified: lld/trunk/ELF/Target.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Target.
> cpp?rev=289072&r1=289071&r2=289072&view=diff
> >> ============================================================
> ==================
> >> --- lld/trunk/ELF/Target.cpp (original)
> >> +++ lld/trunk/ELF/Target.cpp Thu Dec  8 11:18:09 2016
> >> @@ -1320,7 +1320,7 @@ void AArch64TargetInfo::writePlt(uint8_t
> >>    relocateOne(Buf + 8, R_AARCH64_ADD_ABS_LO12_NC, GotEntryAddr);
> >>  }
> >>
> >> -static void updateAArch64Addr(uint8_t *L, uint64_t Imm) {
> >> +static void write32addr(uint8_t *L, uint64_t Imm) {
> >>    uint32_t ImmLo = (Imm & 0x3) << 29;
> >>    uint32_t ImmHi = (Imm & 0x1FFFFC) << 3;
> >>    uint64_t Mask = (0x3 << 29) | (0x1FFFFC << 3);
> >> @@ -1334,8 +1334,8 @@ static uint64_t getBits(uint64_t Val, in
> >>    return (Val >> Start) & Mask;
> >>  }
> >>
> >> -// Update the immediate field in a ldr, str, and add instruction.
> >> -static inline void updateAArch64LdStrAdd(uint8_t *L, uint64_t Imm) {
> >> +// Update the immediate field in a AARCH64 ldr, str, and add
> instruction.
> >
> > This is far less than ideal. Now I need to look at the code to
> > understand what the function does, which wasn't true before.
> > Saving few characters is not worth the gain, IMHO. This is not a
> > StringRef function that's used in hundreds of files in LLVM, it's just
> > a static function used in few places in this file. It's fairly
> > unlikely there are gonna be more uses of it, so I don't see the
> > rationale behind this change. Also, I think the correct spelling is
> > AArch64, and not AARCH64 (but I would confirm with Renato or some of
> > the other ARM folks). Ditto for the other function.
> >
>
> ping.
>
> --
> Davide
>
> "There are no solved problems; there are only problems that are more
> or less solved" -- Henri Poincare
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161213/c863e9fb/attachment-0001.html>


More information about the llvm-commits mailing list