[PATCH] xen/x86: Fix up rules when forcing -mno-sse

[PATCH] xen/x86: Fix up rules when forcing -mno-sse

[Doug Goldstein]

From: Nathan Rossi <nathan.rossi@xilinx.com>

* When forcing -mno-sse specify -mfpmath=387 to avoid the fallback
  warning

Signed-off-by: Nathan Rossi <nathan.rossi@xilinx.com>
Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---

I found this patch when attempting to build Xen master on Yocto and
wanted to make sure to give the original author credit. His patch is
unmodified (save for the commit message note that says it was not
submitted upstream).

---
 xen/arch/x86/Rules.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 4a04a8a..974f8ab 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -35,7 +35,7 @@ x86_64 := y
 shadow-paging ?= y
 bigmem        ?= n
 
-CFLAGS += -mno-red-zone -mno-sse -fpic
+CFLAGS += -mno-red-zone -mno-sse -fpic -mfpmath=387
 CFLAGS += -fno-asynchronous-unwind-tables
 # -fvisibility=hidden reduces -fpic cost, if it's available
 ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n)
-- 
2.1.4

Re: [PATCH] xen/x86: Fix up rules when forcing -mno-sse

[Jan Beulich]

>>> On 12.08.15 at 17:23, <cardoe@cardoe.com> wrote:
> From: Nathan Rossi <nathan.rossi@xilinx.com>
> 
> * When forcing -mno-sse specify -mfpmath=387 to avoid the fallback
>   warning

"the fallback warning" being what? I've never seen any, and I don't
think I've ever heard anyone else say there was one.

Apart from that I don't think -mfpmath=387 is what we want - we'd
better tell the compiler to not use x87 registers at all.

Jan

Re: [PATCH] xen/x86: Fix up rules when forcing -mno-sse

[Doug Goldstein]

On Wed, Aug 12, 2015 at 10:38 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 12.08.15 at 17:23, <cardoe@cardoe.com> wrote:
>> From: Nathan Rossi <nathan.rossi@xilinx.com>
>>
>> * When forcing -mno-sse specify -mfpmath=387 to avoid the fallback
>>   warning
>
> "the fallback warning" being what? I've never seen any, and I don't
> think I've ever heard anyone else say there was one.
>
> Apart from that I don't think -mfpmath=387 is what we want - we'd
> better tell the compiler to not use x87 registers at all.
>
> Jan
>

I guess I jumped the gun a bit. When targeting a corei7 based chip, Yocto uses
the following CFLAGS for the packages it builds:

-march=corei7 -mtune=corei7 -mfpmath=sse -msse4.2

The issue is the -mfpmath value.

-- 
Doug Goldstein

Re: [PATCH] xen/x86: Fix up rules when forcing -mno-sse

[Andrew Cooper]

On 12/08/15 17:03, Doug Goldstein wrote:
> On Wed, Aug 12, 2015 at 10:38 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 12.08.15 at 17:23, <cardoe@cardoe.com> wrote:
>>> From: Nathan Rossi <nathan.rossi@xilinx.com>
>>>
>>> * When forcing -mno-sse specify -mfpmath=387 to avoid the fallback
>>>   warning
>> "the fallback warning" being what? I've never seen any, and I don't
>> think I've ever heard anyone else say there was one.
>>
>> Apart from that I don't think -mfpmath=387 is what we want - we'd
>> better tell the compiler to not use x87 registers at all.

Agreed.

However, a git grep shows:

crypto/vmac.c:1098:    double cpb;
crypto/vmac.c:1158:             
((double)CLOCKS_PER_SEC*speed_lengths[i]*speed_iters[i]));

which I suspect might need fixing.

>>
>> Jan
>>
> I guess I jumped the gun a bit. When targeting a corei7 based chip, Yocto uses
> the following CFLAGS for the packages it builds:
>
> -march=corei7 -mtune=corei7 -mfpmath=sse -msse4.2
>
> The issue is the -mfpmath value.
>

It seems wrong to set -mfpmath=387, as that implies that floating point
maths is ok.

I cant however find an obvious "no fpmath" option.

Perhaps Xen should specifically filter options such as those out from
the environmental CFLAGS

~Andrew

Re: [PATCH] xen/x86: Fix up rules when forcing -mno-sse

[Doug Goldstein]

On Wed, Aug 12, 2015 at 11:12 AM, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> On 12/08/15 17:03, Doug Goldstein wrote:
>> On Wed, Aug 12, 2015 at 10:38 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 12.08.15 at 17:23, <cardoe@cardoe.com> wrote:
>>>> From: Nathan Rossi <nathan.rossi@xilinx.com>
>>>>
>>>> * When forcing -mno-sse specify -mfpmath=387 to avoid the fallback
>>>>   warning
>>> "the fallback warning" being what? I've never seen any, and I don't
>>> think I've ever heard anyone else say there was one.
>>>
>>> Apart from that I don't think -mfpmath=387 is what we want - we'd
>>> better tell the compiler to not use x87 registers at all.
>
> Agreed.
>
> However, a git grep shows:
>
> crypto/vmac.c:1098:    double cpb;
> crypto/vmac.c:1158:
> ((double)CLOCKS_PER_SEC*speed_lengths[i]*speed_iters[i]));
>
> which I suspect might need fixing.
>

Looks like that only comes in when VMAC_RUN_TESTS is defined. That code
is inside a main().

-- 
Doug Goldstein

Re: [PATCH] xen/x86: Fix up rules when forcing -mno-sse

[Nathan Rossi]

On Thu, Aug 13, 2015 at 2:34 AM, Doug Goldstein <cardoe@cardoe.com> wrote:
> On Wed, Aug 12, 2015 at 11:12 AM, Andrew Cooper
> <andrew.cooper3@citrix.com> wrote:
>> On 12/08/15 17:03, Doug Goldstein wrote:
>>> On Wed, Aug 12, 2015 at 10:38 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 12.08.15 at 17:23, <cardoe@cardoe.com> wrote:
>>>>> From: Nathan Rossi <nathan.rossi@xilinx.com>
>>>>>
>>>>> * When forcing -mno-sse specify -mfpmath=387 to avoid the fallback
>>>>>   warning
>>>> "the fallback warning" being what? I've never seen any, and I don't
>>>> think I've ever heard anyone else say there was one.

The fallback warning is just GCC getting conflicting -mfpmath options,
and falls back to 387 arithmetic (but the warning is treated as an
error). You may not see it with newer versions of GCC I have not
tested if the issue still persists. The specific message is:

"warning: SSE instruction set disabled, using 387 arithmetics"

Popped up as an issue with Yocto/OE + meta-virtualization layer. See
https://lists.yoctoproject.org/pipermail/meta-virtualization/2015-February/000862.html.

>>>>
>>>> Apart from that I don't think -mfpmath=387 is what we want - we'd
>>>> better tell the compiler to not use x87 registers at all.

Using "-msoft-float -mno-fp-ret-in-387" might be useful, it will at
least generate linker errors (as the softfp library is not configured
to be linked) for all uses of float/double operations.

>>
>> Agreed.
>>
>> However, a git grep shows:
>>
>> crypto/vmac.c:1098:    double cpb;
>> crypto/vmac.c:1158:
>> ((double)CLOCKS_PER_SEC*speed_lengths[i]*speed_iters[i]));
>>
>> which I suspect might need fixing.
>>
>
> Looks like that only comes in when VMAC_RUN_TESTS is defined. That code
> is inside a main().
>
> --
> Doug Goldstein

Re: [PATCH] xen/x86: Fix up rules when forcing -mno-sse

[Jan Beulich]

>>> On 12.08.15 at 18:12, <andrew.cooper3@citrix.com> wrote:
> On 12/08/15 17:03, Doug Goldstein wrote:
>> On Wed, Aug 12, 2015 at 10:38 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 12.08.15 at 17:23, <cardoe@cardoe.com> wrote:
>>>> From: Nathan Rossi <nathan.rossi@xilinx.com>
>>>>
>>>> * When forcing -mno-sse specify -mfpmath=387 to avoid the fallback
>>>>   warning
>>> "the fallback warning" being what? I've never seen any, and I don't
>>> think I've ever heard anyone else say there was one.
>>>
>>> Apart from that I don't think -mfpmath=387 is what we want - we'd
>>> better tell the compiler to not use x87 registers at all.
> 
> Agreed.
> 
> However, a git grep shows:
> 
> crypto/vmac.c:1098:    double cpb;
> crypto/vmac.c:1158:             
> ((double)CLOCKS_PER_SEC*speed_lengths[i]*speed_iters[i]));
> 
> which I suspect might need fixing.

Not really - the grep didn't show you that this sits inside a main()
function inside #if VMAC_RUN_TESTS.

>> I guess I jumped the gun a bit. When targeting a corei7 based chip, Yocto uses
>> the following CFLAGS for the packages it builds:
>>
>> -march=corei7 -mtune=corei7 -mfpmath=sse -msse4.2
>>
>> The issue is the -mfpmath value.
>>
> 
> It seems wrong to set -mfpmath=387, as that implies that floating point
> maths is ok.
> 
> I cant however find an obvious "no fpmath" option.
> 
> Perhaps Xen should specifically filter options such as those out from
> the environmental CFLAGS

Maybe. But perhaps suppressing such being enforced by a build
system on a package like Xen (the hypervisor part of it, perhaps
for the tools this would be okay) or the kernel should rather be
done at the packaging level. Or we might consider ignoring CFLAGS
passed through the environment (and require overrides to be
done via make command line arguments instead).

Jan