| Message ID | 87bn9nk6mx.fsf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On December 18, 2015 6:29:10 PM GMT+01:00, Nick Clifton <nickc@redhat.com> wrote: >Hi Guys, > > The gcc test gcc.dg/lto/pr55113_0.c is currently generating an ICE > when compiled for x86 and x86_64 targets using the mainline sources > (revision 231805): > ><built-in>: internal compiler error: in layout_type, at >stor-layout.c:2212 >0xaee2c9 layout_type(tree_node*) > /work/sources/gcc/current/gcc/stor-layout.c:2211 >0xd9637b make_vector_type > /work/sources/gcc/current/gcc/tree.c:9858 >0xde6cbd ix86_get_builtin_type > /work/sources/gcc/current/gcc/config/i386/i386.c:29504 >0xde6d90 ix86_get_builtin_func_type > > The problem is that the test enables -fshort-doubles, but the > ix86_get_builtin_func_type still uses the long doubles for the vector > modes of the builtin types. These long doubles have a larger > alignment requirement than their short double equivalents, which > eventually results in the assertion in stor-layout.c being triggered. > > The patch below is a fix for this problem, although I am not sure if > it is the correct fix. The patch selects the corresponding SFmode > variant of the DFmode vector type when flag_short_doubles is enabled. > Maybe a better patch would be to disallow these builtins when using an > ABI breaking option ? > > Checked with no regressions on an x86_64-pc-linux-gnu toolchain. > > OK to apply ? I think the option should be simply removed... >Cheers > Nick > >gcc/ChangeLog >2015-12-18 Nick Clifton <nickc@redhat.com> > > * config/i386/i386.c (ix86_get_builtin_type): Use SFmode variant > of DFmode vector types when -fshort-doubles is enabled. > >Index: gcc/config/i386/i386.c >=================================================================== >--- gcc/config/i386/i386.c (revision 231805) >+++ gcc/config/i386/i386.c (working copy) >@@ -29501,6 +29501,17 @@ > itype = ix86_get_builtin_type (ix86_builtin_type_vect_base[index]); > mode = ix86_builtin_type_vect_mode[index]; > >+ if (flag_short_double) >+ { >+ switch (mode) >+ { >+ case V2DFmode: mode = V2SFmode; break; >+ case V4DFmode: mode = V4SFmode; break; >+ case V8DFmode: mode = V8SFmode; break; >+ default: break; >+ } >+ } >+ > type = build_vector_type_for_mode (itype, mode); > } > else
Hi Richard, >> The patch below is a fix for this problem, although I am not sure if >> it is the correct fix. The patch selects the corresponding SFmode >> variant of the DFmode vector type when flag_short_doubles is enabled. >> Maybe a better patch would be to disallow these builtins when using an >> ABI breaking option ? > I think the option should be simply removed... Tempting - but we are in stage 3... My patch at least fixes the ICE for now. Cheers Nick
On Tue, Dec 22, 2015 at 10:55 AM, Nick Clifton <nickc@redhat.com> wrote: > Hi Richard, > >>> The patch below is a fix for this problem, although I am not sure if >>> it is the correct fix. The patch selects the corresponding SFmode >>> variant of the DFmode vector type when flag_short_doubles is enabled. >>> Maybe a better patch would be to disallow these builtins when using an >>> ABI breaking option ? > > >> I think the option should be simply removed... > > > Tempting - but we are in stage 3... My patch at least fixes the ICE for > now. Yeah, but is it a complete fix for all the -fshort-double issues? Richard. > Cheers > Nick >
On 01/08/2016 10:41 AM, Richard Biener wrote: > On Tue, Dec 22, 2015 at 10:55 AM, Nick Clifton <nickc@redhat.com> wrote: >> Hi Richard, >> >>>> The patch below is a fix for this problem, although I am not sure if >>>> it is the correct fix. The patch selects the corresponding SFmode >>>> variant of the DFmode vector type when flag_short_doubles is enabled. >>>> Maybe a better patch would be to disallow these builtins when using an >>>> ABI breaking option ? >> >> >>> I think the option should be simply removed... >> >> >> Tempting - but we are in stage 3... My patch at least fixes the ICE for >> now. > > Yeah, but is it a complete fix for all the -fshort-double issues? I vote for removing it given that it appears not to have worked for several releases (PR60410). It is a target specific option really (m68hc11 judging by one entry in ChangeLog-2001). Bernd
On 01/08/2016 02:00 PM, Bernd Schmidt wrote: > On 01/08/2016 10:41 AM, Richard Biener wrote: >> On Tue, Dec 22, 2015 at 10:55 AM, Nick Clifton <nickc@redhat.com> wrote: >>> Richard Biener wrote: >>>> I think the option should be simply removed... >>> >>> Tempting - but we are in stage 3... My patch at least fixes the ICE for >>> now. >> >> Yeah, but is it a complete fix for all the -fshort-double issues? > > I vote for removing it given that it appears not to have worked for > several releases (PR60410). It is a target specific option really > (m68hc11 judging by one entry in ChangeLog-2001). So I was looking at removing it, but it turns out t-mips-img tries to build a multilib using -fshort-double. Matthew - is this really required? I tried to build a mips-img-elf compiler, but building libgcc fails with conftest.c:1:0: error: unsupported combination: 'mips32r6' -mno-explicit-relocs /* confdefs.h */ conftest.c:1:0: error: '-mno-gpopt' needs '-mexplicit-relocs' Bernd
Index: gcc/config/i386/i386.c =================================================================== --- gcc/config/i386/i386.c (revision 231805) +++ gcc/config/i386/i386.c (working copy) @@ -29501,6 +29501,17 @@ itype = ix86_get_builtin_type (ix86_builtin_type_vect_base[index]); mode = ix86_builtin_type_vect_mode[index]; + if (flag_short_double) + { + switch (mode) + { + case V2DFmode: mode = V2SFmode; break; + case V4DFmode: mode = V4SFmode; break; + case V8DFmode: mode = V8SFmode; break; + default: break; + } + } + type = build_vector_type_for_mode (itype, mode); } else