NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: lib/59391: unnecessary __PIC__ conditionals clutter .S files
The following reply was made to PR lib/59391; it has been noted by GNATS.
From: Valery Ushakov <uwe%stderr.spb.ru@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc:
Subject: Re: lib/59391: unnecessary __PIC__ conditionals clutter .S files
Date: Sun, 4 May 2025 14:12:57 +0300
> New macros PCREL_GET(rN,label,pclabel) and PCREL_SYM(label,pclabel)
Speaking from sh3 experience (where similar macros have been around
for 20 years), there are two things in this that kinda feel awkward.
The "pclabel" in PCREL_GET(a, label, pclabel) should be hoisted out
ouf the macro, and instead of
PCREL_GET(rN, .Lsym, .LPC0)
one can write just
0: PCREL_GET(rN, .Lsym)
with the natural asm syntax for labels. The label doesn't need to be
hidden inside the macro (unlike, say, "gotsym" in GOT_INITSYM, where
preceding .align gets in the way). I think this lessens the cognitive
burden on the reader. Also, with the label in its natural position
you can also use local labels, b/c in the label position that number
is obviously a local label, while in
PCREL_GET(rN, .Lsym, 0)
that zero is extra confusing.
Second, "PCREL" in the names is a confusing, b/c the end result is to
set rN to the _ABSOLUTE_ address, not to the pc relative offset.
The sbrk.S example
/* get address or offset to __curbrk */
PCREL_GET(r2, .Lcurbrk, .LPIC0)
is the perfect storm - the comment used to refer to the "ldr"
instruction that is now inside the macro. The macro always gets the
address, so the comment is now wrong and (with "PCREL" in the macro
name) doubly confusing.
-uwe
Home |
Main Index |
Thread Index |
Old Index