| Message ID | 20170816103358.GZ17069@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Aug 16, 2017 at 12:33 PM, Marek Polacek <polacek@redhat.com> wrote: > On Mon, Aug 14, 2017 at 10:22:09AM +0200, Richard Biener wrote: >> On Tue, Aug 8, 2017 at 7:23 PM, Marek Polacek <polacek@redhat.com> wrote: >> > On Mon, Aug 07, 2017 at 04:07:49PM +0200, Richard Biener wrote: >> >> On August 7, 2017 11:09:59 AM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote: >> >> >On Mon, Aug 07, 2017 at 10:58:09AM +0200, Jakub Jelinek wrote: >> >> >> On Mon, Aug 07, 2017 at 10:47:51AM +0200, Marek Polacek wrote: >> >> >> > In my recent change I failed to check whether the type domain >> >> >> > of a type is non-NULL and this goof causes crashing on this >> >> >> > testcase. >> >> >> > >> >> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk? >> >> >> > >> >> >> > 2017-08-07 Marek Polacek <polacek@redhat.com> >> >> >> > >> >> >> > PR middle-end/81737 >> >> >> > * fold-const.c (fold_indirect_ref_1): Check type_domain. >> >> >> > >> >> >> > * gcc.dg/pr81737.c: New test. >> >> >> >> >> >> The old code was assuming size_zero_node if type_domain is NULL >> >> >> or TYPE_MIN_VALUE is NULL, which is reasonable for C/C++, but indeed >> >> >might >> >> >> be wrong perhaps for Fortran or Ada. >> >> >> >> It's how the middle-end defines it, so please restore this behavior (sorry for missing this in the review). IIRC there are at least one other 'copy' of the folding somewhere. >> > >> > Sure, this patch should do it: >> > >> > Bootstrapped/regtested on x86_64-linux, ok for trunk? >> > >> > 2017-08-08 Marek Polacek <polacek@redhat.com> >> > >> > PR middle/81695 >> > * fold-const.c (fold_indirect_ref_1): Restore original behavior >> > regarding size_zero_node. >> > >> > diff --git gcc/fold-const.c gcc/fold-const.c >> > index 5a118ca50a1..2c47d1d16a4 100644 >> > --- gcc/fold-const.c >> > +++ gcc/fold-const.c >> > @@ -14109,22 +14109,21 @@ fold_indirect_ref_1 (location_t loc, tree type, tree op0) >> > && type == TREE_TYPE (op00type)) >> > { >> > tree type_domain = TYPE_DOMAIN (op00type); >> > - tree min; >> > + tree min = size_zero_node; >> > if (type_domain != NULL_TREE >> > - && (min = TYPE_MIN_VALUE (type_domain)) >> > + && TYPE_MIN_VALUE (type_domain) >> > && TREE_CODE (min) == INTEGER_CST) >> > + min = TYPE_MIN_VALUE (type_domain); >> >> I think this is wrong for non-INTEGER_CST TYPE_MIN_VALUE. > > Ah, right, another try: > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2017-08-16 Marek Polacek <polacek@redhat.com> > > PR middle/81695 > * fold-const.c (fold_indirect_ref_1): Restore original behavior > regarding size_zero_node. > > diff --git gcc/fold-const.c gcc/fold-const.c > index 5a118ca50a1..0a05da0b49f 100644 > --- gcc/fold-const.c > +++ gcc/fold-const.c > @@ -14109,22 +14109,19 @@ fold_indirect_ref_1 (location_t loc, tree type, tree op0) > && type == TREE_TYPE (op00type)) > { > tree type_domain = TYPE_DOMAIN (op00type); > - tree min; > - if (type_domain != NULL_TREE > - && (min = TYPE_MIN_VALUE (type_domain)) > - && TREE_CODE (min) == INTEGER_CST) > + tree min = size_zero_node; > + if (type_domain && TYPE_MIN_VALUE (type_domain)) > + min = TYPE_MIN_VALUE (type_domain); > + offset_int off = wi::to_offset (op01); > + offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type)); > + offset_int remainder; > + off = wi::divmod_trunc (off, el_sz, SIGNED, &remainder); > + if (remainder == 0) Ok with && TREE_CODE (min) == INTEGER_CST here. Richard. > { > - offset_int off = wi::to_offset (op01); > - offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type)); > - offset_int remainder; > - off = wi::divmod_trunc (off, el_sz, SIGNED, &remainder); > - if (remainder == 0) > - { > - off = off + wi::to_offset (min); > - op01 = wide_int_to_tree (sizetype, off); > - return build4_loc (loc, ARRAY_REF, type, op00, op01, > - NULL_TREE, NULL_TREE); > - } > + off = off + wi::to_offset (min); > + op01 = wide_int_to_tree (sizetype, off); > + return build4_loc (loc, ARRAY_REF, type, op00, op01, > + NULL_TREE, NULL_TREE); > } > } > } > > Marek
diff --git gcc/fold-const.c gcc/fold-const.c index 5a118ca50a1..0a05da0b49f 100644 --- gcc/fold-const.c +++ gcc/fold-const.c @@ -14109,22 +14109,19 @@ fold_indirect_ref_1 (location_t loc, tree type, tree op0) && type == TREE_TYPE (op00type)) { tree type_domain = TYPE_DOMAIN (op00type); - tree min; - if (type_domain != NULL_TREE - && (min = TYPE_MIN_VALUE (type_domain)) - && TREE_CODE (min) == INTEGER_CST) + tree min = size_zero_node; + if (type_domain && TYPE_MIN_VALUE (type_domain)) + min = TYPE_MIN_VALUE (type_domain); + offset_int off = wi::to_offset (op01); + offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type)); + offset_int remainder; + off = wi::divmod_trunc (off, el_sz, SIGNED, &remainder); + if (remainder == 0) { - offset_int off = wi::to_offset (op01); - offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type)); - offset_int remainder; - off = wi::divmod_trunc (off, el_sz, SIGNED, &remainder); - if (remainder == 0) - { - off = off + wi::to_offset (min); - op01 = wide_int_to_tree (sizetype, off); - return build4_loc (loc, ARRAY_REF, type, op00, op01, - NULL_TREE, NULL_TREE); - } + off = off + wi::to_offset (min); + op01 = wide_int_to_tree (sizetype, off); + return build4_loc (loc, ARRAY_REF, type, op00, op01, + NULL_TREE, NULL_TREE); } } }