Patchwork [fortran] PR 45745: [4.6 Regression] ICE in gfc_conv_array_stride

login
register
mail settings
Submitter Mikael Morin
Date Sept. 22, 2010, 10:11 p.m.
Message ID <201009230011.40217.mikael.morin@sfr.fr>
Download mbox | patch
Permalink /patch/65471/
State New
Headers show

Comments

Mikael Morin - Sept. 22, 2010, 10:11 p.m.
Hello,

here is the fix for the regression[PR45745] introduced by the fix for the 
regression[PR45648] introduced by the transpose patches. 

As explained in the PR, while working on PR45648 I noticed the [ss->expr != 
expr] condition was sufficient to distinguish elemental function calls, as 
claimed by a comment [/* Elemental function.  */]. Thus, I dropped the 
additional condition. 
 
However, the code block was also supposed to be hit for (non-elemental) bounds 
intrinsics. 
This restores the additional [ || ss->type != GFC_SS_FUNCTION] condition which 
had been dropped. 

I attach a -b version of the patch (read-only) and the normal one (write-
only). The test is the reduced one, but I can put the original one (not much 
bigger) if someone prefer. 
Regression tested on x86_64-unknown-freebsd8.0. OK for trunk ?

Mikael
2010-09-22  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/45745
	PR fortran/45648
	* trans-array.c (gfc_conv_expr_descriptor): Handle 
	ss->type == GFC_SS_INTRINSIC (for {l,u}bound intrinsics) case.
2010-09-21  Mikael Morin  <mikael@gcc.gnu.org>

	PR fortran/45745
	PR fortran/45648
	* gfortran.dg/vector_subscript_bound_1.f90: New.
Paul Richard Thomas - Sept. 23, 2010, 4:55 a.m.
Dear Mikael,


> As explained in the PR, while working on PR45648 I noticed the [ss->expr !=
> expr] condition was sufficient to distinguish elemental function calls, as
> claimed by a comment [/* Elemental function.  */]. Thus, I dropped the
> additional condition.

I did wonder about that but did not succeed in breaking it.  Then I
became distracted by that stupidity over the loop exit condition :-(

>
> However, the code block was also supposed to be hit for (non-elemental) bounds
> intrinsics.
> This restores the additional [ || ss->type != GFC_SS_FUNCTION] condition which
> had been dropped.

OK

> I attach a -b version of the patch (read-only) and the normal one (write-
> only). The test is the reduced one, but I can put the original one (not much
> bigger) if someone prefer.

No, I think that the reduced test is fine.

> Regression tested on x86_64-unknown-freebsd8.0. OK for trunk ?

OK - thanks for responding so quickly.

It's ironic that it is the tidy-up rather than the fix for PR45648
that caused the regression.....

Cheers

Paul
Mikael Morin - Sept. 23, 2010, 11:13 a.m.
On Thursday 23 September 2010 06:55:31 Paul Richard Thomas wrote:
> Dear Mikael,
> 
> > As explained in the PR, while working on PR45648 I noticed the [ss->expr
> > != expr] condition was sufficient to distinguish elemental function
> > calls, as claimed by a comment [/* Elemental function.  */]. Thus, I
> > dropped the additional condition.
> 
> I did wonder about that but did not succeed in breaking it.  Then I
> became distracted by that stupidity over the loop exit condition :-(
> 
> > However, the code block was also supposed to be hit for (non-elemental)
> > bounds intrinsics.
> > This restores the additional [ || ss->type != GFC_SS_FUNCTION] condition
> > which had been dropped.
> 
> OK
> 
> > I attach a -b version of the patch (read-only) and the normal one (write-
> > only). The test is the reduced one, but I can put the original one (not
> > much bigger) if someone prefer.
> 
> No, I think that the reduced test is fine.
> 
> > Regression tested on x86_64-unknown-freebsd8.0. OK for trunk ?
> 
> OK - thanks for responding so quickly.
> 
> It's ironic that it is the tidy-up rather than the fix for PR45648
> that caused the regression.....
> 
> Cheers
> 
> Paul

Revision 164558
Thanks for reviewing.

Mikael

Patch

Index: trans-array.c
===================================================================
--- trans-array.c	(révision 164525)
+++ trans-array.c	(copie de travail)
@@ -5290,13 +5290,17 @@  gfc_conv_expr_descriptor (gfc_se * se, gfc_expr *
 	  return;
 	}
 
-      if (ss->expr != expr)
+      if (ss->expr != expr || ss->type != GFC_SS_FUNCTION)
 	{
-	  /* Elemental function.  */
-	  gcc_assert ((expr->value.function.esym != NULL
-		       && expr->value.function.esym->attr.elemental)
-		      || (expr->value.function.isym != NULL
-			  && expr->value.function.isym->elemental));
+	  if (ss->expr != expr)
+	    /* Elemental function.  */
+	    gcc_assert ((expr->value.function.esym != NULL
+			 && expr->value.function.esym->attr.elemental)
+			|| (expr->value.function.isym != NULL
+			    && expr->value.function.isym->elemental));
+	  else
+	    gcc_assert (ss->type == GFC_SS_INTRINSIC);
+
 	  need_tmp = 1;
 	  if (expr->ts.type == BT_CHARACTER
 		&& expr->ts.u.cl->length->expr_type != EXPR_CONSTANT)