Patchwork [middle-end] : Fix PR50083: All 32-bit fortran tests fail on 32-bit Solaris

login
register
mail settings
Submitter Uros Bizjak
Date Aug. 25, 2011, 6:55 p.m.
Message ID <CAFULd4b7UVH6fHASJOva-w+NpqKkkJBtk78gpzgtiEyng8KADA@mail.gmail.com>
Download mbox | patch
Permalink /patch/111637/
State New
Headers show

Comments

Uros Bizjak - Aug. 25, 2011, 6:55 p.m.
Hello!

As noted in the PR, we also have to protect conversion from
round->lround for non-TARGET_C99_FUNCTIONS targets. Otherwise, gcc
chokes in fold_fixed_mathfn, trying to canonicalize iround to
(non-existent) lround. It looks to me, that we can trigger the same
problem trying to convert (long long) round -> llround -> lround on
non-TARGET_C99_FUNCTIONS LP64 targets, so this fix probably applies to
other release branches as well.

2011-08-25  Uros Bizjak  <ubizjak@gmail.com>

	PR middle-end/50083
	* convert.c (convert_to_integer) <BUIT_IN_ROUND{,F,L}>: Convert
	only when TARGET_C99_FUNCTIONS.
	<BUILT_IN_NEARBYINT{,F,L}>: Ditto.
	<BUILT_IN_RINT{,F,L}>: Ditto.

Bootstrapped on x86_64-pc-linux-gnu, regtesting in progress.

OK for SVN and 4.6?

Uros.
Richard Guenther - Aug. 26, 2011, 7:05 a.m.
On Thu, 25 Aug 2011, Uros Bizjak wrote:

> Hello!
> 
> As noted in the PR, we also have to protect conversion from
> round->lround for non-TARGET_C99_FUNCTIONS targets. Otherwise, gcc
> chokes in fold_fixed_mathfn, trying to canonicalize iround to
> (non-existent) lround. It looks to me, that we can trigger the same
> problem trying to convert (long long) round -> llround -> lround on
> non-TARGET_C99_FUNCTIONS LP64 targets, so this fix probably applies to
> other release branches as well.
> 
> 2011-08-25  Uros Bizjak  <ubizjak@gmail.com>
> 
> 	PR middle-end/50083
> 	* convert.c (convert_to_integer) <BUIT_IN_ROUND{,F,L}>: Convert
> 	only when TARGET_C99_FUNCTIONS.
> 	<BUILT_IN_NEARBYINT{,F,L}>: Ditto.
> 	<BUILT_IN_RINT{,F,L}>: Ditto.
> 
> Bootstrapped on x86_64-pc-linux-gnu, regtesting in progress.
> 
> OK for SVN and 4.6?

Hmm.  In builtins.c we usually check if the target has support to
expand the builtins directly in case we have named patterns for them.
IMHO these convert.c optimizations belong somewhere else (so that
they trigger for all languages).  Somewhere else these days would
be tree-ssa-forwprop.c.

I'm not asking you to do this move but please consider also doing
the optimization when the target can expand the function directly.

Thanks,
Richard.
Uros Bizjak - Aug. 26, 2011, 7:23 a.m.
On Fri, Aug 26, 2011 at 9:05 AM, Richard Guenther <rguenther@suse.de> wrote:

>> As noted in the PR, we also have to protect conversion from
>> round->lround for non-TARGET_C99_FUNCTIONS targets. Otherwise, gcc
>> chokes in fold_fixed_mathfn, trying to canonicalize iround to
>> (non-existent) lround. It looks to me, that we can trigger the same
>> problem trying to convert (long long) round -> llround -> lround on
>> non-TARGET_C99_FUNCTIONS LP64 targets, so this fix probably applies to
>> other release branches as well.
>>
>> 2011-08-25  Uros Bizjak  <ubizjak@gmail.com>
>>
>>       PR middle-end/50083
>>       * convert.c (convert_to_integer) <BUIT_IN_ROUND{,F,L}>: Convert
>>       only when TARGET_C99_FUNCTIONS.
>>       <BUILT_IN_NEARBYINT{,F,L}>: Ditto.
>>       <BUILT_IN_RINT{,F,L}>: Ditto.
>>
>> Bootstrapped on x86_64-pc-linux-gnu, regtesting in progress.
>>
>> OK for SVN and 4.6?
>
> Hmm.  In builtins.c we usually check if the target has support to
> expand the builtins directly in case we have named patterns for them.
> IMHO these convert.c optimizations belong somewhere else (so that
> they trigger for all languages).  Somewhere else these days would
> be tree-ssa-forwprop.c.
>
> I'm not asking you to do this move but please consider also doing
> the optimization when the target can expand the function directly.

Yes, I know from our previous communication (ilogb handling) that this
whole convert.c part is fishy, but my attached patch fixes the
unwanted conversion in the same way as other similar builtins are
handled.

Uros.
Richard Guenther - Aug. 26, 2011, 7:30 a.m.
On Fri, 26 Aug 2011, Uros Bizjak wrote:

> On Fri, Aug 26, 2011 at 9:05 AM, Richard Guenther <rguenther@suse.de> wrote:
> 
> >> As noted in the PR, we also have to protect conversion from
> >> round->lround for non-TARGET_C99_FUNCTIONS targets. Otherwise, gcc
> >> chokes in fold_fixed_mathfn, trying to canonicalize iround to
> >> (non-existent) lround. It looks to me, that we can trigger the same
> >> problem trying to convert (long long) round -> llround -> lround on
> >> non-TARGET_C99_FUNCTIONS LP64 targets, so this fix probably applies to
> >> other release branches as well.
> >>
> >> 2011-08-25  Uros Bizjak  <ubizjak@gmail.com>
> >>
> >>       PR middle-end/50083
> >>       * convert.c (convert_to_integer) <BUIT_IN_ROUND{,F,L}>: Convert
> >>       only when TARGET_C99_FUNCTIONS.
> >>       <BUILT_IN_NEARBYINT{,F,L}>: Ditto.
> >>       <BUILT_IN_RINT{,F,L}>: Ditto.
> >>
> >> Bootstrapped on x86_64-pc-linux-gnu, regtesting in progress.
> >>
> >> OK for SVN and 4.6?
> >
> > Hmm.  In builtins.c we usually check if the target has support to
> > expand the builtins directly in case we have named patterns for them.
> > IMHO these convert.c optimizations belong somewhere else (so that
> > they trigger for all languages).  Somewhere else these days would
> > be tree-ssa-forwprop.c.
> >
> > I'm not asking you to do this move but please consider also doing
> > the optimization when the target can expand the function directly.
> 
> Yes, I know from our previous communication (ilogb handling) that this
> whole convert.c part is fishy, but my attached patch fixes the
> unwanted conversion in the same way as other similar builtins are
> handled.

Hmm, right, I see that now.

Well, patch is ok then.

Thanks,
Richard.
Uros Bizjak - Aug. 26, 2011, 7:33 a.m.
On Fri, Aug 26, 2011 at 9:30 AM, Richard Guenther <rguenther@suse.de> wrote:

>> >> As noted in the PR, we also have to protect conversion from
>> >> round->lround for non-TARGET_C99_FUNCTIONS targets. Otherwise, gcc
>> >> chokes in fold_fixed_mathfn, trying to canonicalize iround to
>> >> (non-existent) lround. It looks to me, that we can trigger the same
>> >> problem trying to convert (long long) round -> llround -> lround on
>> >> non-TARGET_C99_FUNCTIONS LP64 targets, so this fix probably applies to
>> >> other release branches as well.
>> >>
>> >> 2011-08-25  Uros Bizjak  <ubizjak@gmail.com>
>> >>
>> >>       PR middle-end/50083
>> >>       * convert.c (convert_to_integer) <BUIT_IN_ROUND{,F,L}>: Convert
>> >>       only when TARGET_C99_FUNCTIONS.
>> >>       <BUILT_IN_NEARBYINT{,F,L}>: Ditto.
>> >>       <BUILT_IN_RINT{,F,L}>: Ditto.
>> >>
>> >> Bootstrapped on x86_64-pc-linux-gnu, regtesting in progress.
>> >>
>> >> OK for SVN and 4.6?
>> >
>> > Hmm.  In builtins.c we usually check if the target has support to
>> > expand the builtins directly in case we have named patterns for them.
>> > IMHO these convert.c optimizations belong somewhere else (so that
>> > they trigger for all languages).  Somewhere else these days would
>> > be tree-ssa-forwprop.c.
>> >
>> > I'm not asking you to do this move but please consider also doing
>> > the optimization when the target can expand the function directly.
>>
>> Yes, I know from our previous communication (ilogb handling) that this
>> whole convert.c part is fishy, but my attached patch fixes the
>> unwanted conversion in the same way as other similar builtins are
>> handled.
>
> Hmm, right, I see that now.
>
> Well, patch is ok then.

I will wait for the confirmation from Rainer before committing the patch.

Uros.
Rainer Orth - Aug. 26, 2011, 2:10 p.m.
Uros,

> I will wait for the confirmation from Rainer before committing the patch.

an i386-pc-solaris2.9 bootstrap just finished, and all the failures are
gone.

Thanks.
        Rainer

Patch

Index: convert.c
===================================================================
--- convert.c	(revision 178071)
+++ convert.c	(working copy)
@@ -469,6 +469,9 @@  convert_to_integer (tree type, tree expr)
 	  break;
 
 	CASE_FLT_FN (BUILT_IN_ROUND):
+	  /* Only convert in ISO C99 mode.  */
+	  if (!TARGET_C99_FUNCTIONS)
+	    break;
 	  if (outprec < TYPE_PRECISION (integer_type_node)
 	      || (outprec == TYPE_PRECISION (integer_type_node)
 		  && !TYPE_UNSIGNED (type)))
@@ -487,11 +490,14 @@  convert_to_integer (tree type, tree expr)
 	    break;
 	  /* ... Fall through ...  */
 	CASE_FLT_FN (BUILT_IN_RINT):
+	  /* Only convert in ISO C99 mode.  */
+	  if (!TARGET_C99_FUNCTIONS)
+	    break;
 	  if (outprec < TYPE_PRECISION (integer_type_node)
 	      || (outprec == TYPE_PRECISION (integer_type_node)
 		  && !TYPE_UNSIGNED (type)))
 	    fn = mathfn_built_in (s_intype, BUILT_IN_IRINT);
-	  else if (outprec < TYPE_PRECISION (long_integer_type_node)
+	  else if (outprec == TYPE_PRECISION (long_integer_type_node)
 		   && !TYPE_UNSIGNED (type))
 	    fn = mathfn_built_in (s_intype, BUILT_IN_LRINT);
 	  else if (outprec == TYPE_PRECISION (long_long_integer_type_node)