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: [v3] Add math-inline benchmark


> Siddhesh Poyarekar wrote:
> Sorry, I assumed someone else (Carlos?) was reviewing this and then
> forgot about it.  I've put review comments inline.  The major concern I
> have is that you're duplicating contents of math_private.h to compare
> it against the gcc builtins, which is fine as a first hack if you want
> to avoid changing actual code for the tests, but may not be a good idea
> in the long term.  That is, there is a risk that the inlines may be
> modified but this test doesn't realize that.
> 
> I don't mind this going in as a first revision (with other changes
> recommended below) provided you're willing to remedy this situation in
> the next iteration so that there is at least a failure in running the
> benchtest if the definitions of the inlines change in any way.

I previously posted a patch to enable inlines in math.h which replace the
GLIBC internal inlines. I also posted a patch to rename all uses of
__isinf_ns to isinf so they also use the new inline. I am going to post a 
patch remove all the math_private inlines as they are never used after
these changes.

So these functions are not duplicated, and it is necessary to add a copy
to benchmark the old implementation. Ie. the only duplication is 
EXTRACT_WORDS64, which seems fairly minor.

I've updated my patch with your other comments, see attached.

Wilco

Attachment: 0001-Add-bench-math-inlines.txt
Description: Text document


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