Re: [PATCH] Add flags for powerpc in order to avoid exceptions

[Andrei Borzenkov <arvidjaar@gmail.com>]

В Wed, 8 Oct 2014 20:38:29 -0300
Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com> пишет:

> Mon, Sep 29, 2014 at 10:30:16PM +0400, Andrei Borzenkov wrote:
> > В Sat, 27 Sep 2014 09:06:53 +0400
> > Andrei Borzenkov <arvidjaar@gmail.com> пишет:
> > 
> > > В Fri, 26 Sep 2014 16:29:40 -0300
> > > Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com> пишет:
> > > 
> > > > * conf/Makefile.common [COND_powerpc_ieee1275] (CFLAGS_PLATFORM): Add
> > > > -msoft-float -mno-vsx -mno-altivec.
> > > > 
> > > > Also-By: Adhemerval Zanella <azanella@linux.vnet.ibm.com>
> > > > ---
> > > >  ChangeLog            | 7 +++++++
> > > >  conf/Makefile.common | 3 +++
> > > >  2 files changed, 10 insertions(+)
> > > > 
> > > > diff --git a/ChangeLog b/ChangeLog
> > > > index 192e8bc..40eb90c 100644
> > > > --- a/ChangeLog
> > > > +++ b/ChangeLog
> > > > @@ -1,3 +1,10 @@
> > > > +2014-09-26  Paulo Flabiano Smorigo  <pfsmorigo@linux.vnet.ibm.com>
> > > > +
> > > > +	Add flags for powerpc in order to avoid exceptions
> > > > +
> > > > +	* conf/Makefile.common [COND_powerpc_ieee1275] (CFLAGS_PLATFORM): Add
> > > > +	-msoft-float -mno-vsx -mno-altivec.
> > > > +
> > > >  2014-09-25  Colin Watson  <cjwatson@ubuntu.com>
> > > >  
> > > >  	Fix in-tree --platform=none
> > > > diff --git a/conf/Makefile.common b/conf/Makefile.common
> > > > index 51fbaf9..57c491a 100644
> > > > --- a/conf/Makefile.common
> > > > +++ b/conf/Makefile.common
> > > > @@ -22,6 +22,9 @@ endif
> > > >  if COND_arm64
> > > >    CFLAGS_PLATFORM += -mcmodel=large
> > > >  endif
> > > > +if COND_powerpc_ieee1275
> > > > +  CFLAGS_PLATFORM += -msoft-float -mno-vsx -mno-altivec
> > > > +endif
> > > >  
> > > >  #FIXME: discover and check XEN headers
> > > >  CPPFLAGS_XEN = -I/usr/include
> > > 
> > > Thank you! I confirm that this patch works and does not break grub on
> > > earlier CPU :)
> > > 
> > > There are still differences in generated code, so we probably need to
> > > tune some more flags.
> > 
> > I looked at compiler options for PowerPC and I think instead of trying
> > to catch individual flags which may change with every release we should
> > simply force the most common platform by
> > 
> > +if COND_powerpc_ieee1275
> > +  CFLAGS_PLATFORM += -mcpu=powerpc -mtune=powerpc
> > +endif
> > 
> > 
> > This makes sure result is identical on every platform and provides most
> > generic code that is expected to run on every(?) system.
> > 
> > WDYT?
> 
> Hi Andrei,
> 
> Sorry to take so long to answer your question.
> 
> After an internal discussion with some toolchain experts we found out
> that your approach is good. It works and it's more generic but we think
> that we should change it a little bit:
> 
>  * skip -mtune, not really necessary

Fine if you say so.

>  * add "-msoft-float" to be completly safe.
> 

Is it really necessary? I do not think grub is using floating point
anywhere.

> Something like:
> 
> +if COND_powerpc_ieee1275
> +  CFLAGS_PLATFORM += -mcpu=powerpc -msoft-float
> +endif
> 

Should not we also add -mbig-endian here, and not in configure? This is
really something we always use, not something we need to detect
dynamically.

> This approach is similar to what the linux kernel do.
> 
> In addition, they recomend to set FP MSR value to zero as well.
> 
> Andrei, Vladimir, agree?
> 


Could you resend your patches as discussed and rebased to git master,
so they are easier to review and comment?