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