This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/4] Nios II: elf subdir changes, unsigned divide in ld.so


On Fri, 28 Mar 2014, Chung-Lin Tang wrote:

> (2) A change in elf/dynamic-link.h, to allow a port to define a special
> unsigned integer divide during RTLD_BOOTSTRAP, by defining
> RTLD_BOOTSTRAP_UDIV.
> 
> The rationale is: while Nios II does have optional hardware integer
> divide defined as part of the ISA spec, the current support we're
> working on here is for the common no-divider case, relying on libgcc
> __udivsi3.
> 
> Unfortunately however, Nios II does not have PC-relative calls (only a
> direct transfer call similar to MIPS), so all subroutine calls under PIC
> have to go through the GOT, which... is not available yet during
> RTLD_BOOTSTRAP.
> 
> So the RTLD_BOOTSTRAP_UDIV macro facility allows nios2 to define its own
> inline udiv in dl-machine.h, for use when libgcc is still not accessible.

With this approach, there's a risk of a future GCC version noticing an 
implementation of division and optimizing it back into a library call.  
Did you consider a -minline-divide or similar GCC option that could be 
used for building the dynamic linker (and if so, what was the rationale 
for rejecting it)?  (See below for one possible reason not to use that 
approach.)

>         (R_NIOS2_*): Define Nios II relocations.

ChangeLog entries need to mention each new symbol individually.

> +/* Allow targets that do not have libgcc __udivsi3 available to define
> +   one locally.  */
> +# if defined RTLD_BOOTSTRAP && defined RTLD_BOOTSTRAP_UDIV
> +#  define RTLD_UDIV(A, B) RTLD_BOOTSTRAP_UDIV (A, B)
> +# else
> +#  define RTLD_UDIV(A, B) ((A) / (B))
> +# endif

#ifdef-like conditionals have a risk of typos being quietly unnoticed, as 
discussed recently in the -Wundef discussions, so for a new macro it would 
be better to have it always defined exactly once without any "#if 
defined".  For example, a header dl-udiv.h that defines 
RTLD_BOOTSTRAP_UDIV trivially in the generic version, non-trivially in the 
Nios II version.

The comment here needs to explain the semantics to be provided by the 
macro RTLD_BOOTSTRAP_UDIV.  In particular, can it be assumed (a) that the 
division is always for word-sized values, (b) that it is always exact, (c) 
that it is always dividing by sizeof (ElfW(reloc))?

Given those assumptions, you could do something like

#define RTLD_BOOTSTRAP_UDIV(A, B) \
  ({ \
    _Static_assert ((B) == 12, "bad arguments to RTLD_BOOTSTRAP_UDIV"); \
    ((A) >> 2) * 0xaaaaaaab; \
  })

(untested), to implement exact division in terms of multiplication, if 
that's available inline and smaller / faster than dividing bit-by-bit.  
(And if multiplication isn't inline either, then you can use a few shifts 
and additions for that particular multiplication.)

I'm not saying these other approaches to the division are necessarily 
smaller / faster on Nios II, but if the interface means the division is 
exact then they should at least be considered.  (And if it's exact, but 
the compiler can't tell that, that's a reason against using an option for 
the compiler to inline division.)

Also, it would be worth reviewing the other approaches discussed when this 
issue was raised for the Xtensa port of glibc 
<https://sourceware.org/ml/libc-alpha/2007-04/msg00002.html>.

> +/* Nios II relocations.  */
> +#define R_NIOS2_NONE		0       /* No reloc.  */
> +#define R_NIOS2_S16		1       /* Direct signed 16 bit.  */
> +#define R_NIOS2_U16		2       /* Direct unsigned 16 bit.  */
> +#define R_NIOS2_PCREL16		3       /* PC relative 16 bit.  */
> +#define R_NIOS2_CALL26		4

Each macro should have a comment rather than giving up on comments after 
the first few.

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]