Nick Clifton <ni...@redhat.com> wrote: >Hi Guys, > > I have tracked down a bug reported against the MSP430 (PR > target/59613) which turns out to be a generic problem. The function > get_mode_bounds() in stor-layout.c computes the minimum and maximum > values for a given mode, but it uses the bitsize of the mode not the > precision. This is incorrect if the two values are different, (which > can happen when the target is using partial integer modes), since > values outside of the precision cannot be stored in the mode, no > matter how many bits the mode uses. > > The result, for the MSP430 in LARGE mode, is that calling > get_mode_bounds() on PSImode returns a maximum value of -1 instead of > 1^20. Note - what happens is that get_mode_bounds computes the > maximum as (1 << 31) - 1 and then calls gen_int_mode to create an RTX > for this value. But gen_int_mode calls trunc_int_for_mode which uses > GET_MODE_PRECISION to determine the sign bits and these bits overwrite > some of the zero bits in (1 << 31) - 1 changing it into -1. > > The result of this behaviour is the bug that I was tracking down. > simplify_const_relational_operation() was erroneously discarding a > comparison of a pointer against zero, because get_mode_bounds told it > that the pointer could only have values between 0 and -1... > > The proposed patch is simple - use the mode's precision and not its > bitsize to compute the bounds. This seems like an obvious fix, but > I am not familiar with all of the uses of get_mode_bounds() so maybe > there is a situation where using the bitsize is correct. > > There were no regressions and several fixed test cases with the > msp430-elf toolchain that I was using, and no regressions with an > i686-pc-linux-gnu toolchain. > > OK to apply ?
Ok without the comment (this is obvious) Thanks, Richard. >Cheers > Nick > >PS. Just found out that the same patch was created by Peter Bigot for >a >different PR: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52856 > > >gcc/ChangeLog >2013-12-30 Nick Clifton <ni...@redhat.com> > > PR target/59631 > * stor-layout.c (get_mode_bounds): Use GET_MODE_PRECISION instead > of GET_MODE_BITSIZE. > >Index: stor-layout.c >=================================================================== >--- stor-layout.c (revision 206241) >+++ stor-layout.c (working copy) >@@ -2816,7 +2816,8 @@ > enum machine_mode target_mode, > rtx *mmin, rtx *mmax) > { >- unsigned size = GET_MODE_BITSIZE (mode); >+ /* PR 59613: Use the precision of the mode, not its bitsize. */ >+ unsigned size = GET_MODE_PRECISION (mode); > unsigned HOST_WIDE_INT min_val, max_val; > > gcc_assert (size <= HOST_BITS_PER_WIDE_INT);
