Patchwork [Fortran] PR fortran/46794: Fix ICE with powers of integers

login
register
mail settings
Submitter Daniel Kraft
Date Dec. 3, 2010, 10:45 p.m.
Message ID <4CF97314.4000206@domob.eu>
Download mbox | patch
Permalink /patch/74218/
State New
Headers show

Comments

Daniel Kraft - Dec. 3, 2010, 10:45 p.m.
Hi all,

I'm (a little) back to gfortran ;)  The attached patch fixes the problem
of PR 46794 -- namely that when we call _gfortran_pow_i4_i4 even for
integer operands with kinds 1 / 2, the result was not converted back
properly to the smaller kinds and this ICEd (in certain cases).  The fix
is simply to add an appropriate conversion.  I do not entirely like the
way this is done in the patch (with the two variables and "special
casing"), but don't see a better implementation -- what do you think?

Regression-tested on x86_64-unknown-linux-gnu without failures -- though 
the run somehow looked strange to me (on the compile-farm); I'll try 
again to be sure.  Ok for trunk?

Yours,
Daniel
Tobias Burnus - Dec. 4, 2010, 8:49 a.m.
Dear Daniel,

Daniel Kraft wrote:
> I do not entirely like the way this is done in the patch (with the two 
> variables and "special casing"), but don't see a better implementation 
> -- what do you think?

I think it is OK - at least I have trouble finding a better solution.

> Regression-tested on x86_64-unknown-linux-gnu without failures -- 
> though the run somehow looked strange to me (on the compile-farm); 
> I'll try again to be sure.  Ok for trunk?

OK for the trunk. Can you check whether one needs to likewise for the 
4.5 and 4.4 branch? (I think one should check on source level - the 
verify_tree might not always catch it. For some reasons, it ICEs here 
with 4.4 and 4.6 but not with 4.5; however, I think that's rather by 
chance and not because of a proper casting.)

Tobias
Daniel Kraft - Dec. 4, 2010, 9:35 a.m.
Hi Tobias,

Tobias Burnus wrote:
>> Regression-tested on x86_64-unknown-linux-gnu without failures -- 
>> though the run somehow looked strange to me (on the compile-farm); 
>> I'll try again to be sure.  Ok for trunk?
> 
> OK for the trunk. Can you check whether one needs to likewise for the 
> 4.5 and 4.4 branch? (I think one should check on source level - the 
> verify_tree might not always catch it. For some reasons, it ICEs here 
> with 4.4 and 4.6 but not with 4.5; however, I think that's rather by 
> chance and not because of a proper casting.)

No further problems with the regtest, thanks for the review!  I 
committed as rev. 167453 to trunk.  I will look at the source for 4.4 
and 4.5 accordingly.

Yours,
Daniel
Daniel Kraft - Dec. 4, 2010, 12:21 p.m.
Tobias Burnus wrote:
>> Regression-tested on x86_64-unknown-linux-gnu without failures -- 
>> though the run somehow looked strange to me (on the compile-farm); 
>> I'll try again to be sure.  Ok for trunk?
> 
> OK for the trunk. Can you check whether one needs to likewise for the 
> 4.5 and 4.4 branch? (I think one should check on source level - the 
> verify_tree might not always catch it. For some reasons, it ICEs here 
> with 4.4 and 4.6 but not with 4.5; however, I think that's rather by 
> chance and not because of a proper casting.)

The code in question is (almost) identical for gcc 4.5 and gcc 4.4; the 
literal patch applied cleanly for 4.5 and regtested fine, backported as 
rev. 167454.  Now I'll look at 4.4.

Cheers,
Daniel
Steve Kargl - Dec. 4, 2010, 6:18 p.m.
On Sat, Dec 04, 2010 at 10:35:16AM +0100, Daniel Kraft wrote:
> Hi Tobias,
> 
> Tobias Burnus wrote:
> >>Regression-tested on x86_64-unknown-linux-gnu without failures -- 
> >>though the run somehow looked strange to me (on the compile-farm); 
> >>I'll try again to be sure.  Ok for trunk?
> >
> >OK for the trunk. Can you check whether one needs to likewise for the 
> >4.5 and 4.4 branch? (I think one should check on source level - the 
> >verify_tree might not always catch it. For some reasons, it ICEs here 
> >with 4.4 and 4.6 but not with 4.5; however, I think that's rather by 
> >chance and not because of a proper casting.)
> 
> No further problems with the regtest, thanks for the review!  I 
> committed as rev. 167453 to trunk.  I will look at the source for 4.4 
> and 4.5 accordingly.
> 

Can you fix the test case to be valid Fortran.  k1 and k2
are used uninitialized.

Patch

Index: gcc/testsuite/gfortran.dg/power2.f90
===================================================================
--- gcc/testsuite/gfortran.dg/power2.f90	(revision 0)
+++ gcc/testsuite/gfortran.dg/power2.f90	(revision 0)
@@ -0,0 +1,22 @@ 
+! { dg-do compile }
+! PR fortran/46794
+
+! Check that results of powers of integers with kinds 1 and 2 are
+! correctly converted back; this used to ICE because a conversion
+! from kind 4 to the correct one was missing.
+
+! Contributed by Daniel Kraft, d@domob.eu.
+
+PROGRAM main
+  IMPLICIT NONE
+
+  INTEGER(KIND=1) :: k1
+  INTEGER(KIND=2) :: k2
+
+  k1 = 1_1 + 1_1**k1
+  k2 = 1_2 + 1_2**k2
+
+  k2 = 1_1 + 1_1**k2
+  k2 = 1_1 + 1_2**k1
+  k2 = 1_1 + 1_2**k2
+END PROGRAM main
Index: gcc/fortran/trans-expr.c
===================================================================
--- gcc/fortran/trans-expr.c	(revision 167440)
+++ gcc/fortran/trans-expr.c	(working copy)
@@ -976,6 +976,7 @@ 
   tree gfc_int4_type_node;
   int kind;
   int ikind;
+  int res_ikind_1, res_ikind_2;
   gfc_se lse;
   gfc_se rse;
   tree fndecl = NULL;
@@ -996,6 +997,13 @@ 
 
   gfc_int4_type_node = gfc_get_int_type (4);
 
+  /* In case of integer operands with kinds 1 or 2, we call the integer kind 4
+     library routine.  But in the end, we have to convert the result back
+     if this case applies -- with res_ikind_K, we keep track whether operand K
+     falls into this case.  */
+  res_ikind_1 = -1;
+  res_ikind_2 = -1;
+
   kind = expr->value.op.op1->ts.kind;
   switch (expr->value.op.op2->ts.type)
     {
@@ -1006,6 +1014,7 @@ 
 	case 1:
 	case 2:
 	  rse.expr = convert (gfc_int4_type_node, rse.expr);
+	  res_ikind_2 = ikind;
 	  /* Fall through.  */
 
 	case 4:
@@ -1028,7 +1037,10 @@ 
 	case 1:
 	case 2:
 	  if (expr->value.op.op1->ts.type == BT_INTEGER)
-	    lse.expr = convert (gfc_int4_type_node, lse.expr);
+	    {
+	      lse.expr = convert (gfc_int4_type_node, lse.expr);
+	      res_ikind_1 = kind;
+	    }
 	  else
 	    gcc_unreachable ();
 	  /* Fall through.  */
@@ -1121,6 +1133,15 @@ 
 
   se->expr = build_call_expr_loc (input_location,
 			      fndecl, 2, lse.expr, rse.expr);
+
+  /* Convert the result back if it is of wrong integer kind.  */
+  if (res_ikind_1 != -1 && res_ikind_2 != -1)
+    {
+      /* We want the maximum of both operand kinds as result.  */
+      if (res_ikind_1 < res_ikind_2)
+	res_ikind_1 = res_ikind_2;
+      se->expr = convert (gfc_get_int_type (res_ikind_1), se->expr);
+    }
 }