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: OndÅej BÃlka <neleai at seznam dot cz>
- To: Carlos O'Donell <carlos at redhat dot com>
- 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: Thu, 23 Jul 2015 17:19:30 +0200
- 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> <55AE9944 dot 3070406 at redhat dot com>
On Tue, Jul 21, 2015 at 03:11:00PM -0400, Carlos O'Donell wrote:
> 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.
>
I just asked you to read patch in more critical way so you won't miss
that.
> If at all possible please provide actionable patches that show how you
> think the implementation should be.
>
I already send that previously in thread, read my benchmark.