| Message ID | 20111015212110.GA38875@troutmask.apl.washington.edu |
|---|---|
| State | New |
| Headers | show |
On 10/15/2011 11:21 PM, Steve Kargl wrote: > OK for trunk? > > 2011-10-15 Steven G. Kargl<kargl@gcc.gnu.org> > > * gfortran.dg/ishft_3.f90: Update test. I am not so happy with complete test replacements. How about adding it as new test case? > 2011-10-15 Steven G. Kargl<kargl@gcc.gnu.org> > > * check.c (less_than_bitsize1): Check|shift| <= bit_size(i). > (gfc_check_ishftc): Check|shift| <= bit_size(i) and check > that size is positive. I somehow find less_than_bitsize1's + if (strncmp (arg2, "ISHFT", 5) == 0) not that elegant and would prefer another argument, which tells the function that it should take the absolute value of the argument; however, given that ISHFT seems to be the only function which allows negative arguments, one could also leave it. OK with considering the two remarks. Thanks for the patch! Tobias PS: As you have probably seen, I found a related issue regarding DSHIFTL/DSHIFTR: PR 50753. > Index: testsuite/gfortran.dg/ishft_3.f90 > =================================================================== > --- testsuite/gfortran.dg/ishft_3.f90 (revision 179208) > +++ testsuite/gfortran.dg/ishft_3.f90 (working copy) > @@ -1,11 +1,38 @@ > ! { dg-do compile } > +! PR fortran/50514 > program ishft_3 > - integer i, j > - write(*,*) ishftc( 3, 2, 3 ) > - write(*,*) ishftc( 3, 2, i ) > - write(*,*) ishftc( 3, i, j ) > - write(*,*) ishftc( 3, 128 ) ! { dg-error "exceeds BIT_SIZE of first" } > - write(*,*) ishftc( 3, 0, 128 ) ! { dg-error "exceeds BIT_SIZE of first" } > - write(*,*) ishftc( 3, 0, 0 ) ! { dg-error "Invalid third argument" } > - write(*,*) ishftc( 3, 3, 2 ) ! { dg-error "exceeds third argument" } > + > + implicit none > + > + integer j, m > + > + m = 42 > + ! > + ! These should compile. > + ! > + j = ishft(m, 16) > + j = ishft(m, -16) > + j = ishftc(m, 16) > + j = ishftc(m, -16) > + ! > + ! These should issue an error. > + ! > + j = ishft(m, 640) ! { dg-error "absolute value of SHIFT" } > + j = ishftc(m, 640) ! { dg-error "absolute value of SHIFT" } > + j = ishft(m, -640) ! { dg-error "absolute value of SHIFT" } > + j = ishftc(m, -640) ! { dg-error "absolute value of SHIFT" } > + > + ! abs(SHIFT) must be<= SIZE > + > + j = ishftc(m, 1, 2) > + j = ishftc(m, 1, 2) > + j = ishftc(m, -1, 2) > + j = ishftc(m, -1, 2) > + > + j = ishftc(m, 10, 2)! { dg-error "absolute value of SHIFT" } > + j = ishftc(m, 10, 2)! { dg-error "absolute value of SHIFT" } > + j = ishftc(m, -10, 2)! { dg-error "absolute value of SHIFT" } > + j = ishftc(m, -10, 2)! { dg-error "absolute value of SHIFT" } > + > + j = ishftc(m, 1, -2) ! { dg-error "must be positive" } > end program > > Index: fortran/check.c > =================================================================== > --- fortran/check.c (revision 179208) > +++ fortran/check.c (working copy) > @@ -318,6 +318,22 @@ less_than_bitsize1 (const char *arg1, gf > { > gfc_extract_int (expr2,&i2); > i3 = gfc_validate_kind (BT_INTEGER, expr1->ts.kind, false); > + > + /* For ISHFT[C],|shift| <= bit_size(i). */ > + if (strncmp (arg2, "ISHFT", 5) == 0) > + { > + if (i2< 0) > + i2 = -i2; > + > + if (i2> gfc_integer_kinds[i3].bit_size) > + { > + gfc_error ("The absolute value of SHIFT at %L must be less " > + "than or equal to BIT_SIZE('%s')", > + &expr2->where, arg1); > + return FAILURE; > + } > + } > + > if (or_equal) > { > if (i2> gfc_integer_kinds[i3].bit_size) > @@ -1961,6 +1977,9 @@ gfc_check_ishft (gfc_expr *i, gfc_expr * > || type_check (shift, 1, BT_INTEGER) == FAILURE) > return FAILURE; > > + if (less_than_bitsize1 ("I", i, "ISHFT", shift, true) == FAILURE) > + return FAILURE; > + > return SUCCESS; > } > > @@ -1972,7 +1991,35 @@ gfc_check_ishftc (gfc_expr *i, gfc_expr > || type_check (shift, 1, BT_INTEGER) == FAILURE) > return FAILURE; > > - if (size != NULL&& type_check (size, 2, BT_INTEGER) == FAILURE) > + if (size != NULL) > + { > + int i2, i3; > + > + if (type_check (size, 2, BT_INTEGER) == FAILURE) > + return FAILURE; > + > + if (less_than_bitsize1 ("I", i, "SIZE", size, true) == FAILURE) > + return FAILURE; > + > + gfc_extract_int (size,&i3); > + if (i3<= 0) > + { > + gfc_error ("SIZE at %L must be positive",&size->where); > + return FAILURE; > + } > + > + gfc_extract_int (shift,&i2); > + if (i2< 0) > + i2 = -i2; > + > + if (i2> i3) > + { > + gfc_error ("The absolute value of SHIFT at %L must be less than " > + "or equal to SIZE at %L",&shift->where,&size->where); > + return FAILURE; > + } > + } > + else if (less_than_bitsize1 ("I", i, "ISHFTC", shift, true) == FAILURE) > return FAILURE; > > return SUCCESS;
On Mon, Oct 17, 2011 at 12:22:03PM +0200, Tobias Burnus wrote: > On 10/15/2011 11:21 PM, Steve Kargl wrote: > >OK for trunk? > > > >2011-10-15 Steven G. Kargl<kargl@gcc.gnu.org> > > > > * gfortran.dg/ishft_3.f90: Update test. > > I am not so happy with complete test replacements. How about adding it > as new test case? > Well, the old testcase is %cat ishft_3.f90 ! { dg-do compile } program ishft_3 integer i, j write(*,*) ishftc( 3, 2, 3 ) write(*,*) ishftc( 3, 2, i ) write(*,*) ishftc( 3, i, j ) write(*,*) ishftc( 3, 128 ) ! { dg-error "exceeds BIT_SIZE of first" } write(*,*) ishftc( 3, 0, 128 ) ! { dg-error "exceeds BIT_SIZE of first" } write(*,*) ishftc( 3, 0, 0 ) ! { dg-error "Invalid third argument" } write(*,*) ishftc( 3, 3, 2 ) ! { dg-error "exceeds third argument" } end program 1) It's misnamed. There is no testing of ishft. 2) i and j are undefined, so lines 4 and 5 are invalid. Even if i and j were defined, there is nothing special about those lines in that gfortran generates a function call to a runtime routine. Note, this a "dg-do compile" test. 3) The four dg-error strings would need to be updated to the new error messages. The only line that would survive is the first line, which is covered in the updated testcase. > >2011-10-15 Steven G. Kargl<kargl@gcc.gnu.org> > > > > * check.c (less_than_bitsize1): Check|shift| <= bit_size(i). > > (gfc_check_ishftc): Check|shift| <= bit_size(i) and check > > that size is positive. > > I somehow find less_than_bitsize1's > > + if (strncmp (arg2, "ISHFT", 5) == 0) > > not that elegant and would prefer another argument, which tells the > function that it should take the absolute value of the argument; > however, given that ISHFT seems to be the only function which allows > negative arguments, one could also leave it. > In looking at the other uses of less_than_bitsize1() I could pass arg2=NULL for ISHFT[C], and then the code would become if (arg2 == NULL) { /* Special case for ISHFT[C]. */ Would that be better?
On 10/17/2011 07:02 PM, Steve Kargl wrote: > On Mon, Oct 17, 2011 at 12:22:03PM +0200, Tobias Burnus wrote: >> I am not so happy with complete test replacements. How about adding it >> as new test case? > Well, the old testcase is > [...] > > The only line that would survive is the first line, which > is covered in the updated testcase. Convinced: Do the replacement. >> I somehow find less_than_bitsize1's >> >> + if (strncmp (arg2, "ISHFT", 5) == 0) >> >> not that elegant and would prefer another argument, which tells the >> function that it should take the absolute value of the argument; >> however, given that ISHFT seems to be the only function which allows >> negative arguments, one could also leave it. > In looking at the other uses of less_than_bitsize1() I > could pass arg2=NULL for ISHFT[C], and then the code > would become > > if (arg2 == NULL) { /* Special case for ISHFT[C]. */ > > Would that be better? It would be faster - but not necessarily better. I was rather thinking of adding another argument like "bool abs_ok". However, I think all three solutions are OK. Pick one. Tobias
Index: testsuite/gfortran.dg/ishft_3.f90 =================================================================== --- testsuite/gfortran.dg/ishft_3.f90 (revision 179208) +++ testsuite/gfortran.dg/ishft_3.f90 (working copy) @@ -1,11 +1,38 @@ ! { dg-do compile } +! PR fortran/50514 program ishft_3 - integer i, j - write(*,*) ishftc( 3, 2, 3 ) - write(*,*) ishftc( 3, 2, i ) - write(*,*) ishftc( 3, i, j ) - write(*,*) ishftc( 3, 128 ) ! { dg-error "exceeds BIT_SIZE of first" } - write(*,*) ishftc( 3, 0, 128 ) ! { dg-error "exceeds BIT_SIZE of first" } - write(*,*) ishftc( 3, 0, 0 ) ! { dg-error "Invalid third argument" } - write(*,*) ishftc( 3, 3, 2 ) ! { dg-error "exceeds third argument" } + + implicit none + + integer j, m + + m = 42 + ! + ! These should compile. + ! + j = ishft(m, 16) + j = ishft(m, -16) + j = ishftc(m, 16) + j = ishftc(m, -16) + ! + ! These should issue an error. + ! + j = ishft(m, 640) ! { dg-error "absolute value of SHIFT" } + j = ishftc(m, 640) ! { dg-error "absolute value of SHIFT" } + j = ishft(m, -640) ! { dg-error "absolute value of SHIFT" } + j = ishftc(m, -640) ! { dg-error "absolute value of SHIFT" } + + ! abs(SHIFT) must be <= SIZE + + j = ishftc(m, 1, 2) + j = ishftc(m, 1, 2) + j = ishftc(m, -1, 2) + j = ishftc(m, -1, 2) + + j = ishftc(m, 10, 2)! { dg-error "absolute value of SHIFT" } + j = ishftc(m, 10, 2)! { dg-error "absolute value of SHIFT" } + j = ishftc(m, -10, 2)! { dg-error "absolute value of SHIFT" } + j = ishftc(m, -10, 2)! { dg-error "absolute value of SHIFT" } + + j = ishftc(m, 1, -2) ! { dg-error "must be positive" } end program Index: fortran/check.c =================================================================== --- fortran/check.c (revision 179208) +++ fortran/check.c (working copy) @@ -318,6 +318,22 @@ less_than_bitsize1 (const char *arg1, gf { gfc_extract_int (expr2, &i2); i3 = gfc_validate_kind (BT_INTEGER, expr1->ts.kind, false); + + /* For ISHFT[C], |shift| <= bit_size(i). */ + if (strncmp (arg2, "ISHFT", 5) == 0) + { + if (i2 < 0) + i2 = -i2; + + if (i2 > gfc_integer_kinds[i3].bit_size) + { + gfc_error ("The absolute value of SHIFT at %L must be less " + "than or equal to BIT_SIZE('%s')", + &expr2->where, arg1); + return FAILURE; + } + } + if (or_equal) { if (i2 > gfc_integer_kinds[i3].bit_size) @@ -1961,6 +1977,9 @@ gfc_check_ishft (gfc_expr *i, gfc_expr * || type_check (shift, 1, BT_INTEGER) == FAILURE) return FAILURE; + if (less_than_bitsize1 ("I", i, "ISHFT", shift, true) == FAILURE) + return FAILURE; + return SUCCESS; } @@ -1972,7 +1991,35 @@ gfc_check_ishftc (gfc_expr *i, gfc_expr || type_check (shift, 1, BT_INTEGER) == FAILURE) return FAILURE; - if (size != NULL && type_check (size, 2, BT_INTEGER) == FAILURE) + if (size != NULL) + { + int i2, i3; + + if (type_check (size, 2, BT_INTEGER) == FAILURE) + return FAILURE; + + if (less_than_bitsize1 ("I", i, "SIZE", size, true) == FAILURE) + return FAILURE; + + gfc_extract_int (size, &i3); + if (i3 <= 0) + { + gfc_error ("SIZE at %L must be positive", &size->where); + return FAILURE; + } + + gfc_extract_int (shift, &i2); + if (i2 < 0) + i2 = -i2; + + if (i2 > i3) + { + gfc_error ("The absolute value of SHIFT at %L must be less than " + "or equal to SIZE at %L", &shift->where, &size->where); + return FAILURE; + } + } + else if (less_than_bitsize1 ("I", i, "ISHFTC", shift, true) == FAILURE) return FAILURE; return SUCCESS;