Re: [PATCH] ieee754/dbl-64: Reduce the scope of temporary storage variables

From: Joseph Myers <joseph@codesourcery.com>
To: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Cc: "linux-snps-arc@lists.infradead.org"
	<linux-snps-arc@lists.infradead.org>,
	"libc-alpha@sourceware.org" <libc-alpha@sourceware.org>
Subject: Re: [PATCH] ieee754/dbl-64: Reduce the scope of temporary storage variables
Date: Mon, 11 Nov 2019 22:52:55 +0000	[thread overview]
Message-ID: <alpine.DEB.2.21.1911112247180.30786@digraph.polyomino.org.uk> (raw)
In-Reply-To: <7d22a532-6f19-6b43-2e7a-f6087f658606@synopsys.com>

[-- Attachment #1: Type: text/plain, Size: 2411 bytes --]

On Mon, 11 Nov 2019, Vineet Gupta wrote:

> On 11/11/19 2:33 PM, Joseph Myers wrote:
> > On Mon, 11 Nov 2019, Vineet Gupta wrote:
> >
> >> Functionally it should not result in code gen changes and if at all
> >> those would be better since the scope of those temporaries is greatly
> >> reduced now
> > This feels like the sort of thing where "should not result in code gen 
> > changes" should be tested by running build-many-glibcs.py --strip with 
> > unmodified glibc sources to build a full set of stripped glibc binaries, 
> > saving those binaries and then running build-many-glibcs.py --strip again 
> > and comparing the two sets of shared libraries (something I did a lot of 
> > when reworking how libm function aliases were defined; static libraries 
> > are expected to change because of timestamps, but shared library binaries 
> > can be usefully compared like this).  If the two sets of stripped binaries 
> > are indeed identical, that is strong evidence that the patch is safe; 
> > otherwise, review of the patch will require more detailed inspection of 
> > the types of the arguments to these macros, and the uses of the temporary 
> > variables, at every call site, to make sure that semantics aren't being 
> > changed.
> 
> Ok Understand.  What arch(es) / build options would you want this tested for in
> aforementioned way to get a reasonable confidence ?

The suggestion is two full "build-many-glibcs.py --strip <dir> glibcs" 
runs (one before and one after the change, and comparing the resulting 
binaries), covering all architectures, if you have suitable fast hardware 
for doing such testing (it's quite time-consuming; a single run takes 
about 3 hours wall-clock time on a 16-core / 32-hardware-thread 
Haswell-based Xeon, for example).  (A single "compilers" run can be used 
to build the compilers used for both those "glibcs" runs.)

> > (In any case, please specify when submitting a patch how it was tested.)
> 
> I've currently tested this with build-many-glibcs.py for ARC port only with and
> w/o hard float support being worked on.

If for whatever reason there *are* differences in the stripped glibc 
shared libraries, at least one full run of the glibc testsuite (including 
execution tests) for some architecture will be needed as well to verify 
that the generated code works correctly in at least one configuration.

-- 
Joseph S. Myers
joseph@codesourcery.com

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc