Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Feb 07, 2018 at 04:21:43PM -0500, Jason Merrill wrote:
> On Wed, Feb 7, 2018 at 4:14 PM, Jakub Jelinek wrote:
> > On Wed, Feb 07, 2018 at 03:52:39PM -0500, Jason Merrill wrote:
> >> > E.g. the constexpr function uses
> >> > same_type_ignoring_top_level_qualifiers_p
> >> > instead of == type comparisons, the COMPONENT_REF stuff, ...
> >>
> >> > For poly_* stuff, I think Richard S. wants to introduce it into the FEs
> >> > at
> >> > some point, but I could be wrong; certainly it hasn't been done yet and
> >> > generally, poly*int seems to be a nightmare to deal with.
> >>
> >> Yes, I understand how we got to this point, but having the functions
> >> diverge because of this guideline seems like a mistake. And there
> >> seem to be two ways to avoid the divergence: make an exception to the
> >> guideline, or move the function.
> >
> > Functionally, I think the following patch should turn fold_indirect_ref_1
> > to be equivalent to the patched constexpr.c version (with the known
> > documented differences), so if this is the obstackle for the acceptance
> > of the patch, I can test this.
> >
> > Otherwise, I must say I have no idea how to share the code,
> > same_type_ignoring_qualifiers is only a C++ FE function, so the middle-end
> > can't use it even conditionally, and similarly with the TBAA issues.
>
> Again, can we make an exception and use poly_int in this function
> because it's mirroring a middle-end function?
So like this if it passes bootstrap/regtest? It is kind of bidirectional
merge of changes between the 2 functions, except for intentional differences
(e.g. the same_type_ignoring_top_level_qualifiers_p vs. ==, in_gimple_form
stuff in fold-const.c, the C++ specific empty class etc. handling in
constexpr.c etc.).
2018-01-26 Marek Polacek
Jakub Jelinek
PR c++/83659
* fold-const.c (fold_indirect_ref_1): Use VECTOR_TYPE_P macro.
Formatting fixes. Verify first that tree_fits_poly_int64_p (op01).
Sync some changes from cxx_fold_indirect_ref.
* constexpr.c (cxx_fold_indirect_ref): Sync some changes from
fold_indirect_ref_1, including poly_*int64. Verify first that
tree_fits_poly_int64_p (op01). Formatting fixes.
* g++.dg/torture/pr83659.C: New test.
--- gcc/fold-const.c.jj 2018-01-26 12:43:23.140922419 +0100
+++ gcc/fold-const.c2018-02-08 12:43:50.654727317 +0100
@@ -14115,6 +14115,7 @@ fold_indirect_ref_1 (location_t loc, tre
{
tree op = TREE_OPERAND (sub, 0);
tree optype = TREE_TYPE (op);
+
/* *&CONST_DECL -> to the value of the const decl. */
if (TREE_CODE (op) == CONST_DECL)
return DECL_INITIAL (op);
@@ -14148,12 +14149,13 @@ fold_indirect_ref_1 (location_t loc, tre
&& type == TREE_TYPE (optype))
return fold_build1_loc (loc, REALPART_EXPR, type, op);
/* *(foo *)&vectorfoo => BIT_FIELD_REF */
- else if (TREE_CODE (optype) == VECTOR_TYPE
+ else if (VECTOR_TYPE_P (optype)
&& type == TREE_TYPE (optype))
{
tree part_width = TYPE_SIZE (type);
tree index = bitsize_int (0);
- return fold_build3_loc (loc, BIT_FIELD_REF, type, op, part_width,
index);
+ return fold_build3_loc (loc, BIT_FIELD_REF, type, op, part_width,
+ index);
}
}
@@ -14171,8 +14173,17 @@ fold_indirect_ref_1 (location_t loc, tre
op00type = TREE_TYPE (op00);
/* ((foo*)&vectorfoo)[1] => BIT_FIELD_REF */
- if (TREE_CODE (op00type) == VECTOR_TYPE
- && type == TREE_TYPE (op00type))
+ if (VECTOR_TYPE_P (op00type)
+ && type == TREE_TYPE (op00type)
+ /* POINTER_PLUS_EXPR second operand is sizetype, unsigned,
+but we want to treat offsets with MSB set as negative.
+For the code below negative offsets are invalid and
+TYPE_SIZE of the element is something unsigned, so
+check whether op01 fits into poly_int64, which implies
+it is from 0 to INTTYPE_MAXIMUM (HOST_WIDE_INT), and
+then just use poly_uint64 because we want to treat the
+value as unsigned. */
+ && tree_fits_poly_int64_p (op01))
{
tree part_width = TYPE_SIZE (type);
poly_uint64 max_offset
@@ -14199,16 +14210,16 @@ fold_indirect_ref_1 (location_t loc, tre
&& type == TREE_TYPE (op00type))
{
tree type_domain = TYPE_DOMAIN (op00type);
- tree min = size_zero_node;
+ tree min_val = size_zero_node;
if (type_domain && TYPE_MIN_VALUE (type_domain))
- min = TYPE_MIN_VALUE (type_domain);
+ min_val = TYPE_MIN_VALUE (type_domain);
offset_int off = wi::to_offset (op01);
offset_int el_sz = wi::to_offset (TYPE_SIZE_UNIT (type));
offs
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Thu, Feb 8, 2018 at 6:55 AM, Jakub Jelinek wrote: > On Wed, Feb 07, 2018 at 04:21:43PM -0500, Jason Merrill wrote: >> On Wed, Feb 7, 2018 at 4:14 PM, Jakub Jelinek wrote: >> > On Wed, Feb 07, 2018 at 03:52:39PM -0500, Jason Merrill wrote: >> >> > E.g. the constexpr function uses >> >> > same_type_ignoring_top_level_qualifiers_p >> >> > instead of == type comparisons, the COMPONENT_REF stuff, ... >> >> >> >> > For poly_* stuff, I think Richard S. wants to introduce it into the FEs >> >> > at >> >> > some point, but I could be wrong; certainly it hasn't been done yet and >> >> > generally, poly*int seems to be a nightmare to deal with. >> >> >> >> Yes, I understand how we got to this point, but having the functions >> >> diverge because of this guideline seems like a mistake. And there >> >> seem to be two ways to avoid the divergence: make an exception to the >> >> guideline, or move the function. >> > >> > Functionally, I think the following patch should turn fold_indirect_ref_1 >> > to be equivalent to the patched constexpr.c version (with the known >> > documented differences), so if this is the obstackle for the acceptance >> > of the patch, I can test this. >> > >> > Otherwise, I must say I have no idea how to share the code, >> > same_type_ignoring_qualifiers is only a C++ FE function, so the middle-end >> > can't use it even conditionally, and similarly with the TBAA issues. >> >> Again, can we make an exception and use poly_int in this function >> because it's mirroring a middle-end function? > > So like this if it passes bootstrap/regtest? It is kind of bidirectional > merge of changes between the 2 functions, except for intentional differences > (e.g. the same_type_ignoring_top_level_qualifiers_p vs. ==, in_gimple_form > stuff in fold-const.c, the C++ specific empty class etc. handling in > constexpr.c etc.). Looks good, thanks. Jason
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Thu, Feb 08, 2018 at 12:55:10PM +0100, Jakub Jelinek wrote: > On Wed, Feb 07, 2018 at 04:21:43PM -0500, Jason Merrill wrote: > > On Wed, Feb 7, 2018 at 4:14 PM, Jakub Jelinek wrote: > > > On Wed, Feb 07, 2018 at 03:52:39PM -0500, Jason Merrill wrote: > > >> > E.g. the constexpr function uses > > >> > same_type_ignoring_top_level_qualifiers_p > > >> > instead of == type comparisons, the COMPONENT_REF stuff, ... > > >> > > >> > For poly_* stuff, I think Richard S. wants to introduce it into the > > >> > FEs at > > >> > some point, but I could be wrong; certainly it hasn't been done yet and > > >> > generally, poly*int seems to be a nightmare to deal with. > > >> > > >> Yes, I understand how we got to this point, but having the functions > > >> diverge because of this guideline seems like a mistake. And there > > >> seem to be two ways to avoid the divergence: make an exception to the > > >> guideline, or move the function. > > > > > > Functionally, I think the following patch should turn fold_indirect_ref_1 > > > to be equivalent to the patched constexpr.c version (with the known > > > documented differences), so if this is the obstackle for the acceptance > > > of the patch, I can test this. > > > > > > Otherwise, I must say I have no idea how to share the code, > > > same_type_ignoring_qualifiers is only a C++ FE function, so the middle-end > > > can't use it even conditionally, and similarly with the TBAA issues. > > > > Again, can we make an exception and use poly_int in this function > > because it's mirroring a middle-end function? > > So like this if it passes bootstrap/regtest? It is kind of bidirectional > merge of changes between the 2 functions, except for intentional differences > (e.g. the same_type_ignoring_top_level_qualifiers_p vs. ==, in_gimple_form > stuff in fold-const.c, the C++ specific empty class etc. handling in > constexpr.c etc.). Better than my patch, and also has comments. Thanks and sorry for duplicated effort! Marek
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Thu, Feb 08, 2018 at 10:15:45AM +, Richard Sandiford wrote:
> Jakub Jelinek writes:
> > On Wed, Feb 07, 2018 at 03:23:25PM -0500, Jason Merrill wrote:
> >> On Wed, Feb 7, 2018 at 2:48 PM, Jakub Jelinek wrote:
> >> > On Wed, Feb 07, 2018 at 08:36:31PM +0100, Marek Polacek wrote:
> >> >> > > That was my first patch, but it was rejected:
> >> >> > > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00271.html
> >> >> >
> >> >> > Then should we update fold_indirect_ref_1 to use the new code? Is
> >> >> > there a reason for them to stay out of sync?
> >> >>
> >> >> One of the reasons is that middle end uses poly_uint64 type but the
> >> >> front ends
> >> >> shouldn't use them. So some of these functions will unfortunately
> >> >> differ.
> >> >
> >> > Yeah. Part of the patch makes the two implementations slightly more
> >> > similar, but I have e.g. no idea how to test for poly_uint64 that fits
> >> > also in poly_int64 and the poly_int* stuff makes the two substantially
> >> > different in any case.
> >>
> >> Hmm. Well, that seems rather unfortunate. Why shouldn't the front
> >> ends use them? Can we make an exception for this function because
> >> it's supposed to mirror a middle-end function?
> >> Should we try to push this function back into the middle end?
> >
> > The function comment seems to explain the reasons:
> > /* A less strict version of fold_indirect_ref_1, which requires cv-quals to
> >match. We want to be less strict for simple *& folding; if we have a
> >non-const temporary that we access through a const pointer, that should
> >work. We handle this here rather than change fold_indirect_ref_1
> >because we're dealing with things like ADDR_EXPR of INTEGER_CST which
> >don't really make sense outside of constant expression evaluation. Also
> >we want to allow folding to COMPONENT_REF, which could cause trouble
> >with TBAA in fold_indirect_ref_1.
> >
> >Try to keep this function synced with fold_indirect_ref_1. */
> >
> > E.g. the constexpr function uses same_type_ignoring_top_level_qualifiers_p
> > instead of == type comparisons, the COMPONENT_REF stuff, ...
> >
> > For poly_* stuff, I think Richard S. wants to introduce it into the FEs at
> > some point, but I could be wrong; certainly it hasn't been done yet and
> > generally, poly*int seems to be a nightmare to deal with.
>
> There's no problem with FEs using poly_int now if they want to,
> or if it happens to be more convenient in a particular context
> (which seems to be the case here to avoid divergence). It's more
> that FEs don't need to go out of their way to handle poly_int,
> since the FE can't yet introduce any cases in which the poly_ints
> would be nonconstant.
>
> In practice FEs do already use poly_int directly (and indirectly
> via support routines).
In that case, this patch attemps to synchronize cxx_fold_indirect_ref and
fold_indirect_ref_1 somewhat, especially regarding the poly* stuff.
It also changes if-else-if to if, if, but I can drop that change.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2018-02-08 Marek Polacek
PR c++/83659
* constexpr.c (cxx_fold_indirect_ref): Synchronize with
fold_indirect_ref_1.
* g++.dg/torture/pr83659.C: New test.
diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 93dd8ae049c..6286c7828c6 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -3025,9 +3025,10 @@ cxx_eval_vec_init (const constexpr_ctx *ctx, tree t,
static tree
cxx_fold_indirect_ref (location_t loc, tree type, tree op0, bool *empty_base)
{
- tree sub, subtype;
+ tree sub = op0;
+ tree subtype;
+ poly_uint64 const_op01;
- sub = op0;
STRIP_NOPS (sub);
subtype = TREE_TYPE (sub);
if (!POINTER_TYPE_P (subtype))
@@ -3106,8 +3107,9 @@ cxx_fold_indirect_ref (location_t loc, tree type, tree
op0, bool *empty_base)
return fold_build3 (COMPONENT_REF, type, op, field, NULL_TREE);
}
}
- else if (TREE_CODE (sub) == POINTER_PLUS_EXPR
- && TREE_CODE (TREE_OPERAND (sub, 1)) == INTEGER_CST)
+
+ if (TREE_CODE (sub) == POINTER_PLUS_EXPR
+ && poly_int_tree_p (TREE_OPERAND (sub, 1), &const_op01))
{
tree op00 = TREE_OPERAND (sub, 0);
tree op01 = TREE_OPERAND (sub, 1);
@@ -3124,26 +3126,25 @@ cxx_fold_indirect_ref (location_t loc, tree type, tree
op0, bool *empty_base)
&& (same_type_ignoring_top_level_qualifiers_p
(type, TREE_TYPE (op00type
{
- HOST_WIDE_INT offset = tree_to_shwi (op01);
tree part_width = TYPE_SIZE (type);
- unsigned HOST_WIDE_INT part_widthi = tree_to_shwi
(part_width)/BITS_PER_UNIT;
- unsigned HOST_WIDE_INT indexi = offset * BITS_PER_UNIT;
- tree index = bitsize_int (indexi);
-
- if (known_lt (offset / part_widthi,
- TYPE_VECTOR_SUBPARTS (op00type)))
- return fold_build3_loc (loc,
-
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
Jakub Jelinek writes: > On Wed, Feb 07, 2018 at 03:23:25PM -0500, Jason Merrill wrote: >> On Wed, Feb 7, 2018 at 2:48 PM, Jakub Jelinek wrote: >> > On Wed, Feb 07, 2018 at 08:36:31PM +0100, Marek Polacek wrote: >> >> > > That was my first patch, but it was rejected: >> >> > > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00271.html >> >> > >> >> > Then should we update fold_indirect_ref_1 to use the new code? Is >> >> > there a reason for them to stay out of sync? >> >> >> >> One of the reasons is that middle end uses poly_uint64 type but the >> >> front ends >> >> shouldn't use them. So some of these functions will unfortunately differ. >> > >> > Yeah. Part of the patch makes the two implementations slightly more >> > similar, but I have e.g. no idea how to test for poly_uint64 that fits >> > also in poly_int64 and the poly_int* stuff makes the two substantially >> > different in any case. >> >> Hmm. Well, that seems rather unfortunate. Why shouldn't the front >> ends use them? Can we make an exception for this function because >> it's supposed to mirror a middle-end function? >> Should we try to push this function back into the middle end? > > The function comment seems to explain the reasons: > /* A less strict version of fold_indirect_ref_1, which requires cv-quals to >match. We want to be less strict for simple *& folding; if we have a >non-const temporary that we access through a const pointer, that should >work. We handle this here rather than change fold_indirect_ref_1 >because we're dealing with things like ADDR_EXPR of INTEGER_CST which >don't really make sense outside of constant expression evaluation. Also >we want to allow folding to COMPONENT_REF, which could cause trouble >with TBAA in fold_indirect_ref_1. > >Try to keep this function synced with fold_indirect_ref_1. */ > > E.g. the constexpr function uses same_type_ignoring_top_level_qualifiers_p > instead of == type comparisons, the COMPONENT_REF stuff, ... > > For poly_* stuff, I think Richard S. wants to introduce it into the FEs at > some point, but I could be wrong; certainly it hasn't been done yet and > generally, poly*int seems to be a nightmare to deal with. There's no problem with FEs using poly_int now if they want to, or if it happens to be more convenient in a particular context (which seems to be the case here to avoid divergence). It's more that FEs don't need to go out of their way to handle poly_int, since the FE can't yet introduce any cases in which the poly_ints would be nonconstant. In practice FEs do already use poly_int directly (and indirectly via support routines). Thanks, Richard
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Feb 07, 2018 at 03:52:39PM -0500, Jason Merrill wrote:
> > E.g. the constexpr function uses same_type_ignoring_top_level_qualifiers_p
> > instead of == type comparisons, the COMPONENT_REF stuff, ...
>
> > For poly_* stuff, I think Richard S. wants to introduce it into the FEs at
> > some point, but I could be wrong; certainly it hasn't been done yet and
> > generally, poly*int seems to be a nightmare to deal with.
>
> Yes, I understand how we got to this point, but having the functions
> diverge because of this guideline seems like a mistake. And there
> seem to be two ways to avoid the divergence: make an exception to the
> guideline, or move the function.
Functionally, I think the following patch should turn fold_indirect_ref_1
to be equivalent to the patched constexpr.c version (with the known
documented differences), so if this is the obstackle for the acceptance
of the patch, I can test this.
Otherwise, I must say I have no idea how to share the code,
same_type_ignoring_qualifiers is only a C++ FE function, so the middle-end
can't use it even conditionally, and similarly with the TBAA issues.
--- gcc/fold-const.c.jj 2018-01-26 18:41:34.316410713 +0100
+++ gcc/fold-const.c2018-02-07 22:10:54.958628078 +0100
@@ -14172,7 +14172,16 @@ fold_indirect_ref_1 (location_t loc, tre
/* ((foo*)&vectorfoo)[1] => BIT_FIELD_REF */
if (TREE_CODE (op00type) == VECTOR_TYPE
- && type == TREE_TYPE (op00type))
+ && type == TREE_TYPE (op00type)
+ /* POINTER_PLUS_EXPR second operand is sizetype, unsigned,
+but we want to treat offsets with MSB set as negative.
+For the code below negative offsets are invalid and
+TYPE_SIZE of the element is something unsigned, so
+check whether op01 fits into poly_int64, which implies
+it is from 0 to INTTYPE_MAXIMUM (HOST_WIDE_INT), and
+then just use poly_uint64 because we want to treat the
+value as unsigned. */
+ && tree_fits_poly_int64_p (op01))
{
tree part_width = TYPE_SIZE (type);
poly_uint64 max_offset
Jakub
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Feb 07, 2018 at 08:36:31PM +0100, Marek Polacek wrote: > > > That was my first patch, but it was rejected: > > > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00271.html > > > > Then should we update fold_indirect_ref_1 to use the new code? Is > > there a reason for them to stay out of sync? > > One of the reasons is that middle end uses poly_uint64 type but the front ends > shouldn't use them. So some of these functions will unfortunately differ. Yeah. Part of the patch makes the two implementations slightly more similar, but I have e.g. no idea how to test for poly_uint64 that fits also in poly_int64 and the poly_int* stuff makes the two substantially different in any case. Jakub
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Feb 7, 2018 at 4:14 PM, Jakub Jelinek wrote: > On Wed, Feb 07, 2018 at 03:52:39PM -0500, Jason Merrill wrote: >> > E.g. the constexpr function uses same_type_ignoring_top_level_qualifiers_p >> > instead of == type comparisons, the COMPONENT_REF stuff, ... >> >> > For poly_* stuff, I think Richard S. wants to introduce it into the FEs at >> > some point, but I could be wrong; certainly it hasn't been done yet and >> > generally, poly*int seems to be a nightmare to deal with. >> >> Yes, I understand how we got to this point, but having the functions >> diverge because of this guideline seems like a mistake. And there >> seem to be two ways to avoid the divergence: make an exception to the >> guideline, or move the function. > > Functionally, I think the following patch should turn fold_indirect_ref_1 > to be equivalent to the patched constexpr.c version (with the known > documented differences), so if this is the obstackle for the acceptance > of the patch, I can test this. > > Otherwise, I must say I have no idea how to share the code, > same_type_ignoring_qualifiers is only a C++ FE function, so the middle-end > can't use it even conditionally, and similarly with the TBAA issues. Again, can we make an exception and use poly_int in this function because it's mirroring a middle-end function? Jason
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Feb 7, 2018 at 3:32 PM, Jakub Jelinek wrote: > On Wed, Feb 07, 2018 at 03:23:25PM -0500, Jason Merrill wrote: >> On Wed, Feb 7, 2018 at 2:48 PM, Jakub Jelinek wrote: >> > On Wed, Feb 07, 2018 at 08:36:31PM +0100, Marek Polacek wrote: >> >> > > That was my first patch, but it was rejected: >> >> > > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00271.html >> >> > >> >> > Then should we update fold_indirect_ref_1 to use the new code? Is >> >> > there a reason for them to stay out of sync? >> >> >> >> One of the reasons is that middle end uses poly_uint64 type but the front >> >> ends >> >> shouldn't use them. So some of these functions will unfortunately differ. >> > >> > Yeah. Part of the patch makes the two implementations slightly more >> > similar, but I have e.g. no idea how to test for poly_uint64 that fits >> > also in poly_int64 and the poly_int* stuff makes the two substantially >> > different in any case. >> >> Hmm. Well, that seems rather unfortunate. Why shouldn't the front >> ends use them? Can we make an exception for this function because >> it's supposed to mirror a middle-end function? >> Should we try to push this function back into the middle end? > > The function comment seems to explain the reasons: > /* A less strict version of fold_indirect_ref_1, which requires cv-quals to >match. We want to be less strict for simple *& folding; if we have a >non-const temporary that we access through a const pointer, that should >work. We handle this here rather than change fold_indirect_ref_1 >because we're dealing with things like ADDR_EXPR of INTEGER_CST which >don't really make sense outside of constant expression evaluation. Also >we want to allow folding to COMPONENT_REF, which could cause trouble >with TBAA in fold_indirect_ref_1. > >Try to keep this function synced with fold_indirect_ref_1. */ > > E.g. the constexpr function uses same_type_ignoring_top_level_qualifiers_p > instead of == type comparisons, the COMPONENT_REF stuff, ... > For poly_* stuff, I think Richard S. wants to introduce it into the FEs at > some point, but I could be wrong; certainly it hasn't been done yet and > generally, poly*int seems to be a nightmare to deal with. Yes, I understand how we got to this point, but having the functions diverge because of this guideline seems like a mistake. And there seem to be two ways to avoid the divergence: make an exception to the guideline, or move the function. Jason
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Feb 07, 2018 at 03:23:25PM -0500, Jason Merrill wrote: > On Wed, Feb 7, 2018 at 2:48 PM, Jakub Jelinek wrote: > > On Wed, Feb 07, 2018 at 08:36:31PM +0100, Marek Polacek wrote: > >> > > That was my first patch, but it was rejected: > >> > > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00271.html > >> > > >> > Then should we update fold_indirect_ref_1 to use the new code? Is > >> > there a reason for them to stay out of sync? > >> > >> One of the reasons is that middle end uses poly_uint64 type but the front > >> ends > >> shouldn't use them. So some of these functions will unfortunately differ. > > > > Yeah. Part of the patch makes the two implementations slightly more > > similar, but I have e.g. no idea how to test for poly_uint64 that fits > > also in poly_int64 and the poly_int* stuff makes the two substantially > > different in any case. > > Hmm. Well, that seems rather unfortunate. Why shouldn't the front > ends use them? Can we make an exception for this function because > it's supposed to mirror a middle-end function? > Should we try to push this function back into the middle end? The function comment seems to explain the reasons: /* A less strict version of fold_indirect_ref_1, which requires cv-quals to match. We want to be less strict for simple *& folding; if we have a non-const temporary that we access through a const pointer, that should work. We handle this here rather than change fold_indirect_ref_1 because we're dealing with things like ADDR_EXPR of INTEGER_CST which don't really make sense outside of constant expression evaluation. Also we want to allow folding to COMPONENT_REF, which could cause trouble with TBAA in fold_indirect_ref_1. Try to keep this function synced with fold_indirect_ref_1. */ E.g. the constexpr function uses same_type_ignoring_top_level_qualifiers_p instead of == type comparisons, the COMPONENT_REF stuff, ... For poly_* stuff, I think Richard S. wants to introduce it into the FEs at some point, but I could be wrong; certainly it hasn't been done yet and generally, poly*int seems to be a nightmare to deal with. Jakub
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Feb 7, 2018 at 2:48 PM, Jakub Jelinek wrote: > On Wed, Feb 07, 2018 at 08:36:31PM +0100, Marek Polacek wrote: >> > > That was my first patch, but it was rejected: >> > > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00271.html >> > >> > Then should we update fold_indirect_ref_1 to use the new code? Is >> > there a reason for them to stay out of sync? >> >> One of the reasons is that middle end uses poly_uint64 type but the front >> ends >> shouldn't use them. So some of these functions will unfortunately differ. > > Yeah. Part of the patch makes the two implementations slightly more > similar, but I have e.g. no idea how to test for poly_uint64 that fits > also in poly_int64 and the poly_int* stuff makes the two substantially > different in any case. Hmm. Well, that seems rather unfortunate. Why shouldn't the front ends use them? Can we make an exception for this function because it's supposed to mirror a middle-end function? Should we try to push this function back into the middle end? Jason
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Feb 07, 2018 at 01:23:49PM -0500, Jason Merrill wrote: > On Wed, Feb 7, 2018 at 12:54 PM, Marek Polacek wrote: > > On Wed, Feb 07, 2018 at 12:26:04PM -0500, Jason Merrill wrote: > >> On Fri, Jan 26, 2018 at 6:22 PM, Jakub Jelinek wrote: > >> > On Fri, Jan 26, 2018 at 02:11:19PM +0100, Richard Biener wrote: > >> >> >> POINTER_PLUS_EXPR offets are to be interpreted as signed (ptrdiff_t) > >> >> >> so using uhwi and then performing an unsigned division is wrong code. > >> >> >> See mem_ref_offset how to deal with this (ugh - poly-ints...). > >> >> >> Basically > >> >> >> you have to force the thing to signed. Like just use > >> >> >> > >> >> >> HOST_WIDE_INT offset = TREE_INT_CST_LOW (op01); > >> >> > > >> >> > Does it really matter here though? Any negative offsets there are > >> >> > UB, we > >> >> > should punt on them rather than try to optimize them. > >> >> > As we known op01 is unsigned, if we check that it fits into shwi_p, > >> >> > it means > >> >> > it will be 0 to shwi max and then we can handle it in uhwi too. > >> >> > >> >> Ah, of course. Didn't look up enough context to see what this code > >> >> does in the end ;) > >> >> > >> >> > /* ((foo*)&vectorfoo)[1] => BIT_FIELD_REF */ > >> >> > if (VECTOR_TYPE_P (op00type) > >> >> > && (same_type_ignoring_top_level_qualifiers_p > >> >> > -(type, TREE_TYPE (op00type > >> >> > +(type, TREE_TYPE (op00type))) > >> >> > + && tree_fits_shwi_p (op01)) > >> >> > >> >> nevertheless this appearant "mismatch" deserves a comment (checking > >> >> shwi and using uhwi). > >> > > >> > So like this? > >> > > >> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > >> > >> Why not use the same code as fold_indirect_ref_1 here? > > > > That was my first patch, but it was rejected: > > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00271.html > > Then should we update fold_indirect_ref_1 to use the new code? Is > there a reason for them to stay out of sync? One of the reasons is that middle end uses poly_uint64 type but the front ends shouldn't use them. So some of these functions will unfortunately differ. Marek
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Feb 7, 2018 at 12:54 PM, Marek Polacek wrote: > On Wed, Feb 07, 2018 at 12:26:04PM -0500, Jason Merrill wrote: >> On Fri, Jan 26, 2018 at 6:22 PM, Jakub Jelinek wrote: >> > On Fri, Jan 26, 2018 at 02:11:19PM +0100, Richard Biener wrote: >> >> >> POINTER_PLUS_EXPR offets are to be interpreted as signed (ptrdiff_t) >> >> >> so using uhwi and then performing an unsigned division is wrong code. >> >> >> See mem_ref_offset how to deal with this (ugh - poly-ints...). >> >> >> Basically >> >> >> you have to force the thing to signed. Like just use >> >> >> >> >> >> HOST_WIDE_INT offset = TREE_INT_CST_LOW (op01); >> >> > >> >> > Does it really matter here though? Any negative offsets there are UB, >> >> > we >> >> > should punt on them rather than try to optimize them. >> >> > As we known op01 is unsigned, if we check that it fits into shwi_p, it >> >> > means >> >> > it will be 0 to shwi max and then we can handle it in uhwi too. >> >> >> >> Ah, of course. Didn't look up enough context to see what this code >> >> does in the end ;) >> >> >> >> > /* ((foo*)&vectorfoo)[1] => BIT_FIELD_REF */ >> >> > if (VECTOR_TYPE_P (op00type) >> >> > && (same_type_ignoring_top_level_qualifiers_p >> >> > -(type, TREE_TYPE (op00type >> >> > +(type, TREE_TYPE (op00type))) >> >> > + && tree_fits_shwi_p (op01)) >> >> >> >> nevertheless this appearant "mismatch" deserves a comment (checking >> >> shwi and using uhwi). >> > >> > So like this? >> > >> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? >> >> Why not use the same code as fold_indirect_ref_1 here? > > That was my first patch, but it was rejected: > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00271.html Then should we update fold_indirect_ref_1 to use the new code? Is there a reason for them to stay out of sync? Jason
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Feb 07, 2018 at 12:26:04PM -0500, Jason Merrill wrote: > On Fri, Jan 26, 2018 at 6:22 PM, Jakub Jelinek wrote: > > On Fri, Jan 26, 2018 at 02:11:19PM +0100, Richard Biener wrote: > >> >> POINTER_PLUS_EXPR offets are to be interpreted as signed (ptrdiff_t) > >> >> so using uhwi and then performing an unsigned division is wrong code. > >> >> See mem_ref_offset how to deal with this (ugh - poly-ints...). > >> >> Basically > >> >> you have to force the thing to signed. Like just use > >> >> > >> >> HOST_WIDE_INT offset = TREE_INT_CST_LOW (op01); > >> > > >> > Does it really matter here though? Any negative offsets there are UB, we > >> > should punt on them rather than try to optimize them. > >> > As we known op01 is unsigned, if we check that it fits into shwi_p, it > >> > means > >> > it will be 0 to shwi max and then we can handle it in uhwi too. > >> > >> Ah, of course. Didn't look up enough context to see what this code > >> does in the end ;) > >> > >> > /* ((foo*)&vectorfoo)[1] => BIT_FIELD_REF */ > >> > if (VECTOR_TYPE_P (op00type) > >> > && (same_type_ignoring_top_level_qualifiers_p > >> > -(type, TREE_TYPE (op00type > >> > +(type, TREE_TYPE (op00type))) > >> > + && tree_fits_shwi_p (op01)) > >> > >> nevertheless this appearant "mismatch" deserves a comment (checking > >> shwi and using uhwi). > > > > So like this? > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > Why not use the same code as fold_indirect_ref_1 here? That was my first patch, but it was rejected: https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00271.html Marek
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Fri, Jan 26, 2018 at 6:22 PM, Jakub Jelinek wrote: > On Fri, Jan 26, 2018 at 02:11:19PM +0100, Richard Biener wrote: >> >> POINTER_PLUS_EXPR offets are to be interpreted as signed (ptrdiff_t) >> >> so using uhwi and then performing an unsigned division is wrong code. >> >> See mem_ref_offset how to deal with this (ugh - poly-ints...). Basically >> >> you have to force the thing to signed. Like just use >> >> >> >> HOST_WIDE_INT offset = TREE_INT_CST_LOW (op01); >> > >> > Does it really matter here though? Any negative offsets there are UB, we >> > should punt on them rather than try to optimize them. >> > As we known op01 is unsigned, if we check that it fits into shwi_p, it >> > means >> > it will be 0 to shwi max and then we can handle it in uhwi too. >> >> Ah, of course. Didn't look up enough context to see what this code >> does in the end ;) >> >> > /* ((foo*)&vectorfoo)[1] => BIT_FIELD_REF */ >> > if (VECTOR_TYPE_P (op00type) >> > && (same_type_ignoring_top_level_qualifiers_p >> > -(type, TREE_TYPE (op00type >> > +(type, TREE_TYPE (op00type))) >> > + && tree_fits_shwi_p (op01)) >> >> nevertheless this appearant "mismatch" deserves a comment (checking >> shwi and using uhwi). > > So like this? > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Why not use the same code as fold_indirect_ref_1 here? Jason
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Fri, Jan 26, 2018 at 02:11:19PM +0100, Richard Biener wrote:
> >> POINTER_PLUS_EXPR offets are to be interpreted as signed (ptrdiff_t)
> >> so using uhwi and then performing an unsigned division is wrong code.
> >> See mem_ref_offset how to deal with this (ugh - poly-ints...). Basically
> >> you have to force the thing to signed. Like just use
> >>
> >> HOST_WIDE_INT offset = TREE_INT_CST_LOW (op01);
> >
> > Does it really matter here though? Any negative offsets there are UB, we
> > should punt on them rather than try to optimize them.
> > As we known op01 is unsigned, if we check that it fits into shwi_p, it means
> > it will be 0 to shwi max and then we can handle it in uhwi too.
>
> Ah, of course. Didn't look up enough context to see what this code
> does in the end ;)
>
> > /* ((foo*)&vectorfoo)[1] => BIT_FIELD_REF */
> > if (VECTOR_TYPE_P (op00type)
> > && (same_type_ignoring_top_level_qualifiers_p
> > -(type, TREE_TYPE (op00type
> > +(type, TREE_TYPE (op00type)))
> > + && tree_fits_shwi_p (op01))
>
> nevertheless this appearant "mismatch" deserves a comment (checking
> shwi and using uhwi).
So like this?
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2018-01-26 Marek Polacek
Jakub Jelinek
PR c++/83659
* constexpr.c (cxx_fold_indirect_ref): Use unsigned HOST_WIDE_INT
when computing offsets. Verify first that tree_fits_shwi_p (op01).
Formatting fix.
* g++.dg/torture/pr83659.C: New test.
--- gcc/cp/constexpr.c.jj 2018-01-24 17:18:42.392392254 +0100
+++ gcc/cp/constexpr.c 2018-01-26 18:54:43.991828743 +0100
@@ -3070,7 +3070,8 @@ cxx_fold_indirect_ref (location_t loc, t
{
tree part_width = TYPE_SIZE (type);
tree index = bitsize_int (0);
- return fold_build3_loc (loc, BIT_FIELD_REF, type, op, part_width,
index);
+ return fold_build3_loc (loc, BIT_FIELD_REF, type, op, part_width,
+ index);
}
/* Also handle conversion to an empty base class, which
is represented with a NOP_EXPR. */
@@ -3109,12 +3110,22 @@ cxx_fold_indirect_ref (location_t loc, t
/* ((foo*)&vectorfoo)[1] => BIT_FIELD_REF */
if (VECTOR_TYPE_P (op00type)
- && (same_type_ignoring_top_level_qualifiers_p
- (type, TREE_TYPE (op00type
+ && same_type_ignoring_top_level_qualifiers_p
+ (type, TREE_TYPE (op00type))
+ /* POINTER_PLUS_EXPR second operand is sizetype, unsigned,
+but we want to treat offsets with MSB set as negative.
+For the code below negative offsets are invalid and
+TYPE_SIZE of the element is something unsigned, so
+check whether op01 fits into HOST_WIDE_INT, which implies
+it is from 0 to INTTYPE_MAXIMUM (HOST_WIDE_INT), and
+then just use tree_to_uhwi because we want to treat the
+value as unsigned. */
+ && tree_fits_shwi_p (op01))
{
- HOST_WIDE_INT offset = tree_to_shwi (op01);
+ unsigned HOST_WIDE_INT offset = tree_to_uhwi (op01);
tree part_width = TYPE_SIZE (type);
- unsigned HOST_WIDE_INT part_widthi = tree_to_shwi
(part_width)/BITS_PER_UNIT;
+ unsigned HOST_WIDE_INT part_widthi
+ = tree_to_uhwi (part_width) / BITS_PER_UNIT;
unsigned HOST_WIDE_INT indexi = offset * BITS_PER_UNIT;
tree index = bitsize_int (indexi);
--- gcc/testsuite/g++.dg/torture/pr83659.C.jj 2018-01-26 18:46:40.077572013
+0100
+++ gcc/testsuite/g++.dg/torture/pr83659.C 2018-01-26 18:56:36.822888606
+0100
@@ -0,0 +1,18 @@
+// PR c++/83659
+// { dg-do compile }
+
+typedef int V __attribute__ ((__vector_size__ (16)));
+V a;
+V b[2];
+
+int
+foo ()
+{
+ return reinterpret_cast (&a)[-1] += 1;
+}
+
+int
+bar ()
+{
+ return reinterpret_cast (&a[1])[-1];
+}
Jakub
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Thu, Jan 25, 2018 at 3:45 PM, Jakub Jelinek wrote:
> On Fri, Jan 05, 2018 at 09:52:36AM +0100, Richard Biener wrote:
>> On Wed, Jan 3, 2018 at 5:31 PM, Marek Polacek wrote:
>> > Here we are crashing because cxx_fold_indirect_ref got a POINTER_PLUS_EXPR
>> > with offset > signed HOST_WIDE_INT and we tried to convert it to sHWI.
>> >
>> > The matching code in fold_indirect_ref_1 uses uHWIs so I've followed suit.
>> > But that code now also uses poly_uint64 and I'm not sure if any of the
>> > constexpr.c code should use it, too. But this patch fixes the ICE.
>>
>> POINTER_PLUS_EXPR offets are to be interpreted as signed (ptrdiff_t)
>> so using uhwi and then performing an unsigned division is wrong code.
>> See mem_ref_offset how to deal with this (ugh - poly-ints...). Basically
>> you have to force the thing to signed. Like just use
>>
>> HOST_WIDE_INT offset = TREE_INT_CST_LOW (op01);
>
> Does it really matter here though? Any negative offsets there are UB, we
> should punt on them rather than try to optimize them.
> As we known op01 is unsigned, if we check that it fits into shwi_p, it means
> it will be 0 to shwi max and then we can handle it in uhwi too.
Ah, of course. Didn't look up enough context to see what this code
does in the end ;)
> /* ((foo*)&vectorfoo)[1] => BIT_FIELD_REF */
> if (VECTOR_TYPE_P (op00type)
> && (same_type_ignoring_top_level_qualifiers_p
> -(type, TREE_TYPE (op00type
> +(type, TREE_TYPE (op00type)))
> + && tree_fits_shwi_p (op01))
nevertheless this appearant "mismatch" deserves a comment (checking
shwi and using uhwi).
> {
> - HOST_WIDE_INT offset = tree_to_shwi (op01);
> + unsigned HOST_WIDE_INT offset = tree_to_uhwi (op01);
> tree part_width = TYPE_SIZE (type);
> - unsigned HOST_WIDE_INT part_widthi = tree_to_shwi
> (part_width)/BITS_PER_UNIT;
> + unsigned HOST_WIDE_INT part_widthi
> + = tree_to_uhwi (part_width) / BITS_PER_UNIT;
> unsigned HOST_WIDE_INT indexi = offset * BITS_PER_UNIT;
> tree index = bitsize_int (indexi);
>
> if (known_lt (offset / part_widthi,
> TYPE_VECTOR_SUBPARTS (op00type)))
> return fold_build3_loc (loc,
> BIT_FIELD_REF, type, op00,
> part_width, index);
>
> }
>
> Jakub
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Fri, Jan 05, 2018 at 09:52:36AM +0100, Richard Biener wrote:
> On Wed, Jan 3, 2018 at 5:31 PM, Marek Polacek wrote:
> > Here we are crashing because cxx_fold_indirect_ref got a POINTER_PLUS_EXPR
> > with offset > signed HOST_WIDE_INT and we tried to convert it to sHWI.
> >
> > The matching code in fold_indirect_ref_1 uses uHWIs so I've followed suit.
> > But that code now also uses poly_uint64 and I'm not sure if any of the
> > constexpr.c code should use it, too. But this patch fixes the ICE.
>
> POINTER_PLUS_EXPR offets are to be interpreted as signed (ptrdiff_t)
> so using uhwi and then performing an unsigned division is wrong code.
> See mem_ref_offset how to deal with this (ugh - poly-ints...). Basically
> you have to force the thing to signed. Like just use
>
> HOST_WIDE_INT offset = TREE_INT_CST_LOW (op01);
Does it really matter here though? Any negative offsets there are UB, we
should punt on them rather than try to optimize them.
As we known op01 is unsigned, if we check that it fits into shwi_p, it means
it will be 0 to shwi max and then we can handle it in uhwi too.
/* ((foo*)&vectorfoo)[1] => BIT_FIELD_REF */
if (VECTOR_TYPE_P (op00type)
&& (same_type_ignoring_top_level_qualifiers_p
-(type, TREE_TYPE (op00type
+(type, TREE_TYPE (op00type)))
+ && tree_fits_shwi_p (op01))
{
- HOST_WIDE_INT offset = tree_to_shwi (op01);
+ unsigned HOST_WIDE_INT offset = tree_to_uhwi (op01);
tree part_width = TYPE_SIZE (type);
- unsigned HOST_WIDE_INT part_widthi = tree_to_shwi
(part_width)/BITS_PER_UNIT;
+ unsigned HOST_WIDE_INT part_widthi
+ = tree_to_uhwi (part_width) / BITS_PER_UNIT;
unsigned HOST_WIDE_INT indexi = offset * BITS_PER_UNIT;
tree index = bitsize_int (indexi);
if (known_lt (offset / part_widthi,
TYPE_VECTOR_SUBPARTS (op00type)))
return fold_build3_loc (loc,
BIT_FIELD_REF, type, op00,
part_width, index);
}
Jakub
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Jan 3, 2018 at 5:31 PM, Marek Polacek wrote:
> Here we are crashing because cxx_fold_indirect_ref got a POINTER_PLUS_EXPR
> with offset > signed HOST_WIDE_INT and we tried to convert it to sHWI.
>
> The matching code in fold_indirect_ref_1 uses uHWIs so I've followed suit.
> But that code now also uses poly_uint64 and I'm not sure if any of the
> constexpr.c code should use it, too. But this patch fixes the ICE.
POINTER_PLUS_EXPR offets are to be interpreted as signed (ptrdiff_t)
so using uhwi and then performing an unsigned division is wrong code.
See mem_ref_offset how to deal with this (ugh - poly-ints...). Basically
you have to force the thing to signed. Like just use
HOST_WIDE_INT offset = TREE_INT_CST_LOW (op01);
Richard.
> Bootstrapped/regtested on x86_64-linux, ok for trunk/7?
>
> 2018-01-03 Marek Polacek
>
> PR c++/83659
> * constexpr.c (cxx_fold_indirect_ref): Use unsigned HOST_WIDE_INT
> when computing offsets.
>
> * g++.dg/torture/pr83659.C: New test.
>
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index 1aeacd51810..cf7c994b381 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -3109,9 +3109,10 @@ cxx_fold_indirect_ref (location_t loc, tree type, tree
> op0, bool *empty_base)
> && (same_type_ignoring_top_level_qualifiers_p
> (type, TREE_TYPE (op00type
> {
> - HOST_WIDE_INT offset = tree_to_shwi (op01);
> + unsigned HOST_WIDE_INT offset = tree_to_uhwi (op01);
> tree part_width = TYPE_SIZE (type);
> - unsigned HOST_WIDE_INT part_widthi = tree_to_shwi
> (part_width)/BITS_PER_UNIT;
> + unsigned HOST_WIDE_INT part_widthi
> + = tree_to_uhwi (part_width) / BITS_PER_UNIT;
> unsigned HOST_WIDE_INT indexi = offset * BITS_PER_UNIT;
> tree index = bitsize_int (indexi);
>
> diff --git gcc/testsuite/g++.dg/torture/pr83659.C
> gcc/testsuite/g++.dg/torture/pr83659.C
> index e69de29bb2d..d9f709bb520 100644
> --- gcc/testsuite/g++.dg/torture/pr83659.C
> +++ gcc/testsuite/g++.dg/torture/pr83659.C
> @@ -0,0 +1,11 @@
> +// PR c++/83659
> +// { dg-do compile }
> +
> +typedef int V __attribute__ ((__vector_size__ (16)));
> +V a;
> +
> +int
> +main ()
> +{
> + reinterpret_cast (&a)[-1] += 1;
> +}
>
> Marek
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Jan 03, 2018 at 11:45:55AM -0500, Nathan Sidwell wrote:
> On 01/03/2018 11:31 AM, Marek Polacek wrote:
> > Here we are crashing because cxx_fold_indirect_ref got a POINTER_PLUS_EXPR
> > with offset > signed HOST_WIDE_INT and we tried to convert it to sHWI.
> >
> > The matching code in fold_indirect_ref_1 uses uHWIs so I've followed suit.
> > But that code now also uses poly_uint64 and I'm not sure if any of the
> > constexpr.c code should use it, too. But this patch fixes the ICE.
>
> This doesn't look right to me, but it doesn't help that the test case
> invokes UB.
Hmm, like I said, I just followed fold_indirect_ref_1 so I was pretty confident
that this is not a totally wrong thing to do ;).
> > +typedef int V __attribute__ ((__vector_size__ (16)));
> > +V a;
> > +
> > +int
> > +main ()
> > +{
> > + reinterpret_cast (&a)[-1] += 1;
> > +}
>
> one could make this code well formed with (I think)
>
> typedef int V __attribute__ ((__vector_size__ (16)));
> V a[2];
>
> int main ()
> {
>return reinterpret_cast (&a[1])[-1];
> }
>
> That should access the final element of the a[0] vector.
Unfortunately, this doesn't trigger the ICE.
Marek
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On Wed, Jan 03, 2018 at 05:40:32PM +, Richard Sandiford wrote: > Marek Polacek writes: > > Here we are crashing because cxx_fold_indirect_ref got a POINTER_PLUS_EXPR > > with offset > signed HOST_WIDE_INT and we tried to convert it to sHWI. > > > > The matching code in fold_indirect_ref_1 uses uHWIs so I've followed suit. > > But that code now also uses poly_uint64 and I'm not sure if any of the > > constexpr.c code should use it, too. > > The frontend isn't supposed to see any poly_ints (yet), so it's OK to > keep on using INTEGER_CST accessors like tree_to_shwi and tree_to_uhwi. Thanks, that's good to know. Marek
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
Marek Polacek writes: > Here we are crashing because cxx_fold_indirect_ref got a POINTER_PLUS_EXPR > with offset > signed HOST_WIDE_INT and we tried to convert it to sHWI. > > The matching code in fold_indirect_ref_1 uses uHWIs so I've followed suit. > But that code now also uses poly_uint64 and I'm not sure if any of the > constexpr.c code should use it, too. The frontend isn't supposed to see any poly_ints (yet), so it's OK to keep on using INTEGER_CST accessors like tree_to_shwi and tree_to_uhwi. Thanks, Richard
Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
On 01/03/2018 11:31 AM, Marek Polacek wrote:
Here we are crashing because cxx_fold_indirect_ref got a POINTER_PLUS_EXPR
with offset > signed HOST_WIDE_INT and we tried to convert it to sHWI.
The matching code in fold_indirect_ref_1 uses uHWIs so I've followed suit.
But that code now also uses poly_uint64 and I'm not sure if any of the
constexpr.c code should use it, too. But this patch fixes the ICE.
This doesn't look right to me, but it doesn't help that the test case
invokes UB.
+typedef int V __attribute__ ((__vector_size__ (16)));
+V a;
+
+int
+main ()
+{
+ reinterpret_cast (&a)[-1] += 1;
+}
one could make this code well formed with (I think)
typedef int V __attribute__ ((__vector_size__ (16)));
V a[2];
int main ()
{
return reinterpret_cast (&a[1])[-1];
}
That should access the final element of the a[0] vector.
nathan
--
Nathan Sidwell
