| Message ID | 4CF97314.4000206@domob.eu |
|---|---|
| State | New |
| Headers | show |
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
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
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
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.
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); + } }