[Bug target/68028] [6/7/8 regression] Compilation error "lto1: error: target attribute or pragma changes single precision floating point" with LTO on PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68028
--- Comment #12 from rguenther at suse dot de ---
On Mon, 5 Feb 2018, nickc at redhat dot com wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68028
>
> --- Comment #11 from Nick Clifton ---
> Hi Richard,
>
> > If the backend doesn't support mixing of -msingle-float/-mno-single-float
> > within a compilation unit then this will only work if the user didn't mix
> > TUs
> > with conflicting setting at link-time. So the following will not be
> > diagnosed
> > but will instead have conflicting IL accepted silently with your
> > patch (with unknown effect):
> >
> >> gcc -c t1.c -flto -msingle-float
> >> gcc -c t2.c -flto -mdouble-float
> >> gcc t1.o t2.o -flto
>
> But even without the patch, if the user does the above, but leaves out
> the final -flto then the problem will still not be diagnosed:
>
> gcc -c t1.c -flto -msingle-float
> gcc -c t2.c -flto -mdouble-float
> gcc t1.o t2.o
>
> That is, unless the linker detects and reports this problem. Which it does.
> There are tags recorded in the build notes which specify the precision of
> floating point used, and the linker will complain if these do not match.
>
> So, since the linker will report conflicts there is no need for the LTO
> compiler to do so. Especially when, as this bug report shows, it gets
> the warning wrong under some circumstances.
>
> So I still think that my patch is viable. Comments ?
I'll leave that to the target maintainers to decide -- just wanted to
note that the flags can get mixed by the user via different TUs. Note
that with your patch what you likely get then is simply forcing
a random -m{single,double}-float on the LTO TU. You might be lucky
enough for WPA to order ltrans units in just a way that the linker
will not complain in the end but the flags are not in effect in a way
the user intended.
So I think the UNKNOWN initialized variant would be safer, overriding
exactly once and only diagnosing mismatches.
[Bug target/68028] [6/7/8 regression] Compilation error "lto1: error: target attribute or pragma changes single precision floating point" with LTO on PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68028 --- Comment #11 from Nick Clifton --- Hi Richard, > If the backend doesn't support mixing of -msingle-float/-mno-single-float > within a compilation unit then this will only work if the user didn't mix TUs > with conflicting setting at link-time. So the following will not be diagnosed > but will instead have conflicting IL accepted silently with your > patch (with unknown effect): > >> gcc -c t1.c -flto -msingle-float >> gcc -c t2.c -flto -mdouble-float >> gcc t1.o t2.o -flto But even without the patch, if the user does the above, but leaves out the final -flto then the problem will still not be diagnosed: gcc -c t1.c -flto -msingle-float gcc -c t2.c -flto -mdouble-float gcc t1.o t2.o That is, unless the linker detects and reports this problem. Which it does. There are tags recorded in the build notes which specify the precision of floating point used, and the linker will complain if these do not match. So, since the linker will report conflicts there is no need for the LTO compiler to do so. Especially when, as this bug report shows, it gets the warning wrong under some circumstances. So I still think that my patch is viable. Comments ? Cheers Nick
[Bug target/68028] [6/7/8 regression] Compilation error "lto1: error: target attribute or pragma changes single precision floating point" with LTO on PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68028
--- Comment #10 from Richard Biener ---
(In reply to Nick Clifton from comment #9)
> Created attachment 43318 [details]
> Proposed patch
>
> Hi Guys,
>
> Richard was right - there is code that sets extra flags based upon the
> setting of -mcpu. In fact it is just before the code he mentioned:
>
> /* For the E500 family of cores, reset the single/double FP flags to let us
> check that they remain constant across attributes or pragmas. */
>
> switch (rs6000_cpu)
> {
> case PROCESSOR_PPC8540:
> case PROCESSOR_PPC8548:
> case PROCESSOR_PPCE500MC:
> case PROCESSOR_PPCE500MC64:
> case PROCESSOR_PPCE5500:
> case PROCESSOR_PPCE6500:
> rs6000_single_float = 0;
> rs6000_double_float = 0;
> break;
>
> default:
> break;
> }
>
> This has lead me to propose the attached patch. Basically what it does is to
> tell the rs6000 backend that when it is operating in LTO mode, it should
> trust
> the function attributes and not the command line.
>
> My assumption here is that if there are any discrepancies between real
> function attributes (ie not ones generated for LTO) and the command line,
> then these will have been detected and reported during the original
> compilation.
>
> What do people think ? Is the patch OK ?
If the backend doesn't support mixing of -msingle-float/-mno-single-float
within
a compilation unit then this will only work if the user didn't mix TUs
with conflicting setting at link-time. So the following will not be diagnosed
but will instead have conflicting IL accepted silently with your
patch (with unknown effect):
> gcc -c t1.c -flto -msingle-float
> gcc -c t2.c -flto -mdouble-float
> gcc t1.o t2.o -flto
so I think it's better to have rs6000_single_float/rs6000_double_float
initialized to for example -1 and allow changing that a _single_ time,
verifying all following uses will match.
But I don't see why mixing shouldn't be possible? Maybe the target wants
to instead limit inlining of functions with differing settings instead?
If the caller/callee uses FP? (see i386.c:ix86_can_inline_p use of
cgraph_node::get (callee))->fp_expressions to guard similar things).
[Bug target/68028] [6/7/8 regression] Compilation error "lto1: error: target attribute or pragma changes single precision floating point" with LTO on PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68028
Nick Clifton changed:
What|Removed |Added
CC||nickc at gcc dot gnu.org
--- Comment #9 from Nick Clifton ---
Created attachment 43318
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43318=edit
Proposed patch
Hi Guys,
Richard was right - there is code that sets extra flags based upon the setting
of -mcpu. In fact it is just before the code he mentioned:
/* For the E500 family of cores, reset the single/double FP flags to let us
check that they remain constant across attributes or pragmas. */
switch (rs6000_cpu)
{
case PROCESSOR_PPC8540:
case PROCESSOR_PPC8548:
case PROCESSOR_PPCE500MC:
case PROCESSOR_PPCE500MC64:
case PROCESSOR_PPCE5500:
case PROCESSOR_PPCE6500:
rs6000_single_float = 0;
rs6000_double_float = 0;
break;
default:
break;
}
This has lead me to propose the attached patch. Basically what it does is to
tell the rs6000 backend that when it is operating in LTO mode, it should trust
the function attributes and not the command line.
My assumption here is that if there are any discrepancies between real
function attributes (ie not ones generated for LTO) and the command line,
then these will have been detected and reported during the original
compilation.
What do people think ? Is the patch OK ?
Cheers
Nick
