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] Add generic HAVE_RM_CTX implementation


Ping

As for an additional reviewer, Joseph could you please have a look?

Wilco

-----Original Message-----
From: Wilco [mailto:wdijkstr@arm.com] 
Sent: 13 May 2014 14:35
To: 'Siddhesh Poyarekar'
Cc: Siddhesh Poyarekar; GNU C Library; Marcus Shawcroft
Subject: RE: [PATCH] Add generic HAVE_RM_CTX implementation

> Siddhesh Poyarekar wrote:
> On 12 May 2014 17:32, Wilco <wdijkstr@arm.com> wrote:
> > Marcus (AArch64) already reviewed it.
> 
> I don't see his review on this thread; is it on another thread?

This was an internal review before the patch goes public.

> > I agree we should avoid PLTs for internal calls. However I'm certain
> > most targets wouldn't build if I used __fegetenv etc. libm_hidden_ver
> > is used inconsistently, so this is yet another area that needs a cleanup.
> 
> Fair enough.  We can look at it as a separate cleanup then.
> 
> > Solving that wasn't the purpose of my patch - in fact it is consistent
> > with the rest of math_private.h.
> >
> > Btw can we agree on what the rules are? Eg.
> >
> > * Every exported symbol should have a libm_hidden_ver.
> > * All internal calls must use the hidden symbol.
> 
> An exported symbol should have a hidden alias if it is called
> internally (and internal calls should use that hidden symbol), to
> avoid the extra PLT dereference in the internal call.  So it is not
> necessary to define a hidden alias for *every* exported symbol.

It may not be necessary but it would be the only way to avoid inconsistencies.

Note it turns out the fenv functions called by math_private.h already use direct 
calls. So libm_hidden_def appears to be sufficient to bypass PLTs, and the 
libm_hidden_ver is only required to avoid the namespace issue Joseph mentioned.

I've attatched the new version. Besides the formatting I blocked the annoying
undefined HAVE_RM_CTX warnings.

Cheers,
Wilco

Attachment: Enable-HAVE_RM_CTX-using-a-default-implementation.patch
Description: Binary data


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