On Nov 2, 2013, at 3:13 AM, Richard Sandiford <rdsandif...@googlemail.com> wrote: > I decided to lump these together since the problems were the same. > There were some typos in the real_to_integer invocation, while changing: > > /* There must be no padding. */ > if (!host_integerp (TYPE_SIZE (type), 1) > || (tree_low_cst (TYPE_SIZE (type), 1) > != count * GET_MODE_BITSIZE (*modep))) > return -1; > > to: > > if (!tree_fits_uhwi_p (TYPE_SIZE (type)) > || (tree_to_uhwi (TYPE_SIZE (type)) > != count * GET_MODE_BITSIZE (*modep))) > return -1; > > introduced a signed/unsigned warning. > > Tested on aarch64-linux-gnueabi & arm-linux-gnueabi and applied as > obvious. > > Thanks, > Richard > > > Index: gcc/config/aarch64/aarch64.c > =================================================================== > --- gcc/config/aarch64/aarch64.c (revision 204311) > +++ gcc/config/aarch64/aarch64.c (working copy) > @@ -6030,9 +6030,7 @@ > - tree_to_uhwi (TYPE_MIN_VALUE (index))); > > /* There must be no padding. */ > - if (!tree_fits_uhwi_p (TYPE_SIZE (type)) > - || (tree_to_uhwi (TYPE_SIZE (type)) > - != count * GET_MODE_BITSIZE (*modep))) > + if (wi::ne_p (TYPE_SIZE (type), count * GET_MODE_BITSIZE (*modep))) > return -1;
So, one of the review comments concerns this type of change. The specific comment was from David on rs6000 point #5. My (our) question is, doesn't Ada have non-INTEGER_CST TYPE_SIZE (type), and the old code had this type of check: bool tree_fits_uhwi_p (const_tree t) { return (t != NULL_TREE && TREE_CODE (t) == INTEGER_CST && TREE_INT_CST_HIGH (t) == 0); } to ensure that things that are not INTEGER_CSTs return -1. In the new code, won't this just call wi::ne_p, and die? I'm not an Ada person, so, I don't know if my fears are founded, and I don't claim to know all the checks that happen in the callers before this point.