This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2] Add math-inline benchmark
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: OndÅej BÃlka <neleai at seznam dot cz>
- Cc: Wilco Dijkstra <wdijkstr at arm dot com>, "'Siddhesh Poyarekar'" <siddhesh at redhat dot com>, "'GNU C Library'" <libc-alpha at sourceware dot org>
- Date: Tue, 21 Jul 2015 15:11:00 -0400
- Subject: Re: [PATCH v2] Add math-inline benchmark
- Authentication-results: sourceware.org; auth=none
- References: <002001d0bfb8$b36fa330$1a4ee990$ at com> <55A907F6 dot 8000504 at redhat dot com> <20150717143406 dot GG19592 at spoyarek dot pnq dot redhat dot com> <002c01d0c30a$8f23c600$ad6b5200$ at com> <55AD4AC6 dot 5070401 at redhat dot com> <20150721113502 dot GA3415 at domone> <55AE7789 dot 2010704 at redhat dot com> <20150721173507 dot GA22893 at domone>
On 07/21/2015 01:35 PM, OndÅej BÃlka wrote:
> On Tue, Jul 21, 2015 at 12:47:05PM -0400, Carlos O'Donell wrote:
>> On 07/21/2015 07:35 AM, OndÅej BÃlka wrote:
>>>> One nit, OK to commit with that fixed.
>>>>
>>> No Carlos, this isn't ok. You need to do better review as this patch has
>>> still some issues.
>>
>> It is incremental progress for code that has no immediate API or ABI impact.
>>
>> Therefore I judge it to be OK.
>>
>> I expect Wilco to improve it incrementally, and he has already agreed to
>> remove the test that has little value.
>>
> Then you should say it so. There is additional problem that it doesn't
> measure isnormal inline at all so somebody could pick that and find that
> builtin doesn't improve isnormal just because of typo here. This is
> relatively harmless here but don't have to be.
My apologies, I will endeavour to be more clear next time I give consensus
for a patch.
If I or Wilco missed something the next step is to respond to the original
email with a detailed response about what is missing. Wilco has responded
to the question about isnormal, please respond to his answer and take it
from there to see if we need to change the test.
If at all possible please provide actionable patches that show how you
think the implementation should be.
Cheers,
Carlos.