[RESEND] toolchain: use -ffp-contract=off on MIPS in the wrapper when needed

Message ID 20180308121430.20013-1-ezequiel@vanguardiasur.com.ar
State Changes Requested
Headers show
Series
  • [RESEND] toolchain: use -ffp-contract=off on MIPS in the wrapper when needed
Related show

Commit Message

Ezequiel Garcia March 8, 2018, 12:14 p.m.
From: Johannes Schmitz <johannes.schmitz1@gmail.com>

This fix is necessary for to build for MIPS, for example for the MIPS
XBurst architecture used on ci20 boards.

GCC has replaced (no)mfused-madd with ffp-contract.
Find more details and a long discussion at
https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00876.html

Signed-off-by: Johannes Schmitz <johannes.schmitz1@gmail.com>
Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
Ci20 builds are currently broken without this patch.

 toolchain/toolchain-wrapper.c  | 3 ---
 toolchain/toolchain-wrapper.mk | 4 ++++
 2 files changed, 4 insertions(+), 3 deletions(-)

Comments

Baruch Siach March 8, 2018, 12:33 p.m. | #1
Hi Ezequiel,

On Thu, Mar 08, 2018 at 09:14:30AM -0300, Ezequiel Garcia wrote:
> From: Johannes Schmitz <johannes.schmitz1@gmail.com>
> 
> This fix is necessary for to build for MIPS, for example for the MIPS
> XBurst architecture used on ci20 boards.
> 
> GCC has replaced (no)mfused-madd with ffp-contract.
> Find more details and a long discussion at
> https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00876.html
> 
> Signed-off-by: Johannes Schmitz <johannes.schmitz1@gmail.com>
> Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
> Ci20 builds are currently broken without this patch.
> 
>  toolchain/toolchain-wrapper.c  | 3 ---
>  toolchain/toolchain-wrapper.mk | 4 ++++
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
> index 2928ea42d0e6..04b263199547 100644
> --- a/toolchain/toolchain-wrapper.c
> +++ b/toolchain/toolchain-wrapper.c
> @@ -79,9 +79,6 @@ static char *predef_args[] = {
>  #ifdef BR_OMIT_LOCK_PREFIX
>  	"-Wa,-momit-lock-prefix=yes",
>  #endif
> -#ifdef BR_NO_FUSED_MADD

Since you remove this macro reference, ...

> -	"-mno-fused-madd",
> -#endif
>  #ifdef BR_BINFMT_FLAT
>  	"-Wl,-elf2flt",
>  #endif
> diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
> index 7f72a0cadec9..7faa033f605c 100644
> --- a/toolchain/toolchain-wrapper.mk
> +++ b/toolchain/toolchain-wrapper.mk
> @@ -28,8 +28,12 @@ endif
>  
>  # Avoid FPU bug on XBurst CPUs
>  ifeq ($(BR2_mips_xburst),y)
> +ifeq ($(BR2_TOOLCHAIN_GCC_AT_LEAST_4_6),y)
> +TOOLCHAIN_WRAPPER_ARGS += -DBR_FP_CONTRACT_OFF
> +else
>  TOOLCHAIN_WRAPPER_ARGS += -DBR_NO_FUSED_MADD

... why not remove it here as well?

>  endif
> +endif
>  
>  ifeq ($(BR2_CCACHE_USE_BASEDIR),y)
>  TOOLCHAIN_WRAPPER_ARGS += -DBR_CCACHE_BASEDIR='"$(BASE_DIR)"'

baruch
Ezequiel Garcia March 9, 2018, 1:38 p.m. | #2
Hi Baruch,

On 8 March 2018 at 09:33, Baruch Siach <baruch@tkos.co.il> wrote:
> Hi Ezequiel,
>
> On Thu, Mar 08, 2018 at 09:14:30AM -0300, Ezequiel Garcia wrote:
>> From: Johannes Schmitz <johannes.schmitz1@gmail.com>
>>
>> This fix is necessary for to build for MIPS, for example for the MIPS
>> XBurst architecture used on ci20 boards.
>>
>> GCC has replaced (no)mfused-madd with ffp-contract.
>> Find more details and a long discussion at
>> https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00876.html
>>
>> Signed-off-by: Johannes Schmitz <johannes.schmitz1@gmail.com>
>> Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
>> ---
>> Ci20 builds are currently broken without this patch.
>>
>>  toolchain/toolchain-wrapper.c  | 3 ---
>>  toolchain/toolchain-wrapper.mk | 4 ++++
>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
>> index 2928ea42d0e6..04b263199547 100644
>> --- a/toolchain/toolchain-wrapper.c
>> +++ b/toolchain/toolchain-wrapper.c
>> @@ -79,9 +79,6 @@ static char *predef_args[] = {
>>  #ifdef BR_OMIT_LOCK_PREFIX
>>       "-Wa,-momit-lock-prefix=yes",
>>  #endif
>> -#ifdef BR_NO_FUSED_MADD
>
> Since you remove this macro reference, ...
>
>> -     "-mno-fused-madd",
>> -#endif
>>  #ifdef BR_BINFMT_FLAT
>>       "-Wl,-elf2flt",
>>  #endif
>> diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
>> index 7f72a0cadec9..7faa033f605c 100644
>> --- a/toolchain/toolchain-wrapper.mk
>> +++ b/toolchain/toolchain-wrapper.mk
>> @@ -28,8 +28,12 @@ endif
>>
>>  # Avoid FPU bug on XBurst CPUs
>>  ifeq ($(BR2_mips_xburst),y)
>> +ifeq ($(BR2_TOOLCHAIN_GCC_AT_LEAST_4_6),y)
>> +TOOLCHAIN_WRAPPER_ARGS += -DBR_FP_CONTRACT_OFF
>> +else
>>  TOOLCHAIN_WRAPPER_ARGS += -DBR_NO_FUSED_MADD
>
> ... why not remove it here as well?
>

OK, so now I actually looked at this patch, and I think that it's
all wrong:

1. BR_FP_CONTRACT_OFF is not used anywhere.
2. BR_NO_FUSED_MADD usage is removed,
although it's still defined for GCC < 4.6.

Johannes, want to give a shot at sending a correct version?
Probably we just need to add:

#ifdef BR_FP_CONTRACT_OFF
    "-ffp-contract=off",
#endif

somewhere?
Johannes Schmitz March 9, 2018, 2:52 p.m. | #3
2018-03-09 14:38 GMT+01:00 Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>:

> Hi Baruch,
>
> On 8 March 2018 at 09:33, Baruch Siach <baruch@tkos.co.il> wrote:
> > Hi Ezequiel,
> >
> > On Thu, Mar 08, 2018 at 09:14:30AM -0300, Ezequiel Garcia wrote:
> >> From: Johannes Schmitz <johannes.schmitz1@gmail.com>
> >>
> >> This fix is necessary for to build for MIPS, for example for the MIPS
> >> XBurst architecture used on ci20 boards.
> >>
> >> GCC has replaced (no)mfused-madd with ffp-contract.
> >> Find more details and a long discussion at
> >> https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00876.html
> >>
> >> Signed-off-by: Johannes Schmitz <johannes.schmitz1@gmail.com>
> >> Tested-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> >> ---
> >> Ci20 builds are currently broken without this patch.
> >>
> >>  toolchain/toolchain-wrapper.c  | 3 ---
> >>  toolchain/toolchain-wrapper.mk | 4 ++++
> >>  2 files changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/toolchain/toolchain-wrapper.c
> b/toolchain/toolchain-wrapper.c
> >> index 2928ea42d0e6..04b263199547 100644
> >> --- a/toolchain/toolchain-wrapper.c
> >> +++ b/toolchain/toolchain-wrapper.c
> >> @@ -79,9 +79,6 @@ static char *predef_args[] = {
> >>  #ifdef BR_OMIT_LOCK_PREFIX
> >>       "-Wa,-momit-lock-prefix=yes",
> >>  #endif
> >> -#ifdef BR_NO_FUSED_MADD
> >
> > Since you remove this macro reference, ...
> >
> >> -     "-mno-fused-madd",
> >> -#endif
> >>  #ifdef BR_BINFMT_FLAT
> >>       "-Wl,-elf2flt",
> >>  #endif
> >> diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/
> toolchain-wrapper.mk
> >> index 7f72a0cadec9..7faa033f605c 100644
> >> --- a/toolchain/toolchain-wrapper.mk
> >> +++ b/toolchain/toolchain-wrapper.mk
> >> @@ -28,8 +28,12 @@ endif
> >>
> >>  # Avoid FPU bug on XBurst CPUs
> >>  ifeq ($(BR2_mips_xburst),y)
> >> +ifeq ($(BR2_TOOLCHAIN_GCC_AT_LEAST_4_6),y)
> >> +TOOLCHAIN_WRAPPER_ARGS += -DBR_FP_CONTRACT_OFF
> >> +else
> >>  TOOLCHAIN_WRAPPER_ARGS += -DBR_NO_FUSED_MADD
> >
> > ... why not remove it here as well?
> >
>
> OK, so now I actually looked at this patch, and I think that it's
> all wrong:
>
> 1. BR_FP_CONTRACT_OFF is not used anywhere.
> 2. BR_NO_FUSED_MADD usage is removed,
> although it's still defined for GCC < 4.6.
>
> Johannes, want to give a shot at sending a correct version?
> Probably we just need to add:
>
> #ifdef BR_FP_CONTRACT_OFF
>     "-ffp-contract=off",
> #endif
>
> somewhere?
> --
> Ezequiel García, VanguardiaSur
> www.vanguardiasur.com.ar
>
Hello,
I will give it a try.

Regards
Johannes
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2018-03-09 14:38 GMT+01:00 Ezequiel Garcia <span dir="ltr">&lt;<a href="mailto:ezequiel@vanguardiasur.com.ar" target="_blank">ezequiel@vanguardiasur.com.ar</a>&gt;</span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Baruch,<br>
<div><div class="gmail-h5"><br>
On 8 March 2018 at 09:33, Baruch Siach &lt;<a href="mailto:baruch@tkos.co.il">baruch@tkos.co.il</a>&gt; wrote:<br>
&gt; Hi Ezequiel,<br>
&gt;<br>
&gt; On Thu, Mar 08, 2018 at 09:14:30AM -0300, Ezequiel Garcia wrote:<br>
&gt;&gt; From: Johannes Schmitz &lt;<a href="mailto:johannes.schmitz1@gmail.com">johannes.schmitz1@gmail.com</a>&gt;<br>
&gt;&gt;<br>
&gt;&gt; This fix is necessary for to build for MIPS, for example for the MIPS<br>
&gt;&gt; XBurst architecture used on ci20 boards.<br>
&gt;&gt;<br>
&gt;&gt; GCC has replaced (no)mfused-madd with ffp-contract.<br>
&gt;&gt; Find more details and a long discussion at<br>
&gt;&gt; <a href="https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00876.html" rel="noreferrer" target="_blank">https://gcc.gnu.org/ml/gcc-<wbr>patches/2015-06/msg00876.html</a><br>
&gt;&gt;<br>
&gt;&gt; Signed-off-by: Johannes Schmitz &lt;<a href="mailto:johannes.schmitz1@gmail.com">johannes.schmitz1@gmail.com</a>&gt;<br>
&gt;&gt; Tested-by: Ezequiel Garcia &lt;<a href="mailto:ezequiel@vanguardiasur.com.ar">ezequiel@vanguardiasur.com.ar</a><wbr>&gt;<br>
&gt;&gt; ---<br>
&gt;&gt; Ci20 builds are currently broken without this patch.<br>
&gt;&gt;<br>
&gt;&gt;  toolchain/toolchain-wrapper.c  | 3 ---<br>
&gt;&gt;  toolchain/<a href="http://toolchain-wrapper.mk" rel="noreferrer" target="_blank">toolchain-wrapper.mk</a> | 4 ++++<br>
&gt;&gt;  2 files changed, 4 insertions(+), 3 deletions(-)<br>
&gt;&gt;<br>
&gt;&gt; diff --git a/toolchain/toolchain-wrapper.<wbr>c b/toolchain/toolchain-wrapper.<wbr>c<br>
&gt;&gt; index 2928ea42d0e6..04b263199547 100644<br>
&gt;&gt; --- a/toolchain/toolchain-wrapper.<wbr>c<br>
&gt;&gt; +++ b/toolchain/toolchain-wrapper.<wbr>c<br>
&gt;&gt; @@ -79,9 +79,6 @@ static char *predef_args[] = {<br>
&gt;&gt;  #ifdef BR_OMIT_LOCK_PREFIX<br>
&gt;&gt;       &quot;-Wa,-momit-lock-prefix=yes&quot;,<br>
&gt;&gt;  #endif<br>
&gt;&gt; -#ifdef BR_NO_FUSED_MADD<br>
&gt;<br>
&gt; Since you remove this macro reference, ...<br>
&gt;<br>
&gt;&gt; -     &quot;-mno-fused-madd&quot;,<br>
&gt;&gt; -#endif<br>
&gt;&gt;  #ifdef BR_BINFMT_FLAT<br>
&gt;&gt;       &quot;-Wl,-elf2flt&quot;,<br>
&gt;&gt;  #endif<br>
&gt;&gt; diff --git a/toolchain/<a href="http://toolchain-wrapper.mk" rel="noreferrer" target="_blank">toolchain-wrapper.<wbr>mk</a> b/toolchain/<a href="http://toolchain-wrapper.mk" rel="noreferrer" target="_blank">toolchain-wrapper.<wbr>mk</a><br>
&gt;&gt; index 7f72a0cadec9..7faa033f605c 100644<br>
&gt;&gt; --- a/toolchain/<a href="http://toolchain-wrapper.mk" rel="noreferrer" target="_blank">toolchain-wrapper.<wbr>mk</a><br>
&gt;&gt; +++ b/toolchain/<a href="http://toolchain-wrapper.mk" rel="noreferrer" target="_blank">toolchain-wrapper.<wbr>mk</a><br>
&gt;&gt; @@ -28,8 +28,12 @@ endif<br>
&gt;&gt;<br>
&gt;&gt;  # Avoid FPU bug on XBurst CPUs<br>
&gt;&gt;  ifeq ($(BR2_mips_xburst),y)<br>
&gt;&gt; +ifeq ($(BR2_TOOLCHAIN_GCC_AT_LEAST_<wbr>4_6),y)<br>
&gt;&gt; +TOOLCHAIN_WRAPPER_ARGS += -DBR_FP_CONTRACT_OFF<br>
&gt;&gt; +else<br>
&gt;&gt;  TOOLCHAIN_WRAPPER_ARGS += -DBR_NO_FUSED_MADD<br>
&gt;<br>
&gt; ... why not remove it here as well?<br>
&gt;<br>
<br>
</div></div>OK, so now I actually looked at this patch, and I think that it&#39;s<br>
all wrong:<br>
<br>
1. BR_FP_CONTRACT_OFF is not used anywhere.<br>
2. BR_NO_FUSED_MADD usage is removed,<br>
although it&#39;s still defined for GCC &lt; 4.6.<br>
<br>
Johannes, want to give a shot at sending a correct version?<br>
Probably we just need to add:<br>
<br>
#ifdef BR_FP_CONTRACT_OFF<br>
    &quot;-ffp-contract=off&quot;,<br>
#endif<br>
<br>
somewhere?<br>
<span class="gmail-HOEnZb"><font color="#888888">--<br>
Ezequiel García, VanguardiaSur<br>
<a href="http://www.vanguardiasur.com.ar" rel="noreferrer" target="_blank">www.vanguardiasur.com.ar</a><br>
</font></span></blockquote></div>Hello,<br>I will give it a try.<br><br></div><div class="gmail_extra">Regards<br></div><div class="gmail_extra">Johannes<br></div><div class="gmail_extra"><br></div></div>

Patch

diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
index 2928ea42d0e6..04b263199547 100644
--- a/toolchain/toolchain-wrapper.c
+++ b/toolchain/toolchain-wrapper.c
@@ -79,9 +79,6 @@  static char *predef_args[] = {
 #ifdef BR_OMIT_LOCK_PREFIX
 	"-Wa,-momit-lock-prefix=yes",
 #endif
-#ifdef BR_NO_FUSED_MADD
-	"-mno-fused-madd",
-#endif
 #ifdef BR_BINFMT_FLAT
 	"-Wl,-elf2flt",
 #endif
diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
index 7f72a0cadec9..7faa033f605c 100644
--- a/toolchain/toolchain-wrapper.mk
+++ b/toolchain/toolchain-wrapper.mk
@@ -28,8 +28,12 @@  endif
 
 # Avoid FPU bug on XBurst CPUs
 ifeq ($(BR2_mips_xburst),y)
+ifeq ($(BR2_TOOLCHAIN_GCC_AT_LEAST_4_6),y)
+TOOLCHAIN_WRAPPER_ARGS += -DBR_FP_CONTRACT_OFF
+else
 TOOLCHAIN_WRAPPER_ARGS += -DBR_NO_FUSED_MADD
 endif
+endif
 
 ifeq ($(BR2_CCACHE_USE_BASEDIR),y)
 TOOLCHAIN_WRAPPER_ARGS += -DBR_CCACHE_BASEDIR='"$(BASE_DIR)"'