From: Marek Polacek <polacek@redhat.com>
To: Jakub Jelinek <jakub@redhat.com>,
Jason Merrill <jason@redhat.com>,
Richard Biener <richard.guenther@gmail.com>,
GCC Patches <gcc-patches@gcc.gnu.org>,
Nathan Sidwell <nathan@acm.org>,
richard.sandiford@linaro.org
Subject: Re: C++ PATCH to fix ICE with vector expr folding (PR c++/83659)
Date: Thu, 08 Feb 2018 11:53:00 -0000 [thread overview]
Message-ID: <20180208115255.GG2608@redhat.com> (raw)
In-Reply-To: <877ernpz1q.fsf@linaro.org>
On Thu, Feb 08, 2018 at 10:15:45AM +0000, Richard Sandiford wrote:
> Jakub Jelinek <jakub@redhat.com> 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 <jakub@redhat.com> 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 <polacek@redhat.com>
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,
- BIT_FIELD_REF, type, op00,
- part_width, index);
-
+ poly_uint64 max_offset
+ = (tree_to_uhwi (part_width) / BITS_PER_UNIT
+ * TYPE_VECTOR_SUBPARTS (op00type));
+ if (known_lt (const_op01, max_offset))
+ {
+ tree index = bitsize_int (const_op01 * BITS_PER_UNIT);
+ return fold_build3_loc (loc,
+ BIT_FIELD_REF, type, op00,
+ part_width, index);
+ }
}
/* ((foo*)&complexfoo)[1] => __imag__ complexfoo */
else if (TREE_CODE (op00type) == COMPLEX_TYPE
&& (same_type_ignoring_top_level_qualifiers_p
(type, TREE_TYPE (op00type))))
{
- tree size = TYPE_SIZE_UNIT (type);
- if (tree_int_cst_equal (size, op01))
+ if (known_eq (wi::to_poly_offset (TYPE_SIZE_UNIT (type)),
+ const_op01))
return fold_build1_loc (loc, IMAGPART_EXPR, type, op00);
}
/* ((foo *)&fooarray)[1] => fooarray[1] */
@@ -3191,10 +3192,11 @@ cxx_fold_indirect_ref (location_t loc, tree type, tree op0, bool *empty_base)
}
}
}
+
/* *(foo *)fooarrptr => (*fooarrptr)[0] */
- else if (TREE_CODE (TREE_TYPE (subtype)) == ARRAY_TYPE
- && (same_type_ignoring_top_level_qualifiers_p
- (type, TREE_TYPE (TREE_TYPE (subtype)))))
+ if (TREE_CODE (TREE_TYPE (subtype)) == ARRAY_TYPE
+ && (same_type_ignoring_top_level_qualifiers_p
+ (type, TREE_TYPE (TREE_TYPE (subtype)))))
{
tree type_domain;
tree min_val = size_zero_node;
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,18 @@
+// PR c++/83659
+// { dg-do compile }
+
+typedef int V __attribute__ ((__vector_size__ (16)));
+V a;
+V b[2];
+
+int
+foo ()
+{
+ return reinterpret_cast <int *> (&a)[-1] += 1;
+}
+
+int
+bar ()
+{
+ return reinterpret_cast <int *> (&a[1])[-1];
+}
Marek
prev parent reply other threads:[~2018-02-08 11:53 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-03 16:31 Marek Polacek
2018-01-03 16:46 ` Nathan Sidwell
2018-01-03 18:00 ` Marek Polacek
2018-01-03 17:40 ` Richard Sandiford
2018-01-03 17:45 ` Marek Polacek
2018-01-05 8:52 ` Richard Biener
2018-01-25 15:10 ` Jakub Jelinek
2018-01-26 13:25 ` Richard Biener
2018-01-26 23:31 ` Jakub Jelinek
2018-02-07 17:26 ` Jason Merrill
2018-02-07 17:54 ` Marek Polacek
2018-02-07 18:24 ` Jason Merrill
2018-02-07 19:36 ` Marek Polacek
2018-02-07 21:43 ` Jakub Jelinek
2018-02-07 20:23 ` Jason Merrill
2018-02-07 20:43 ` Jakub Jelinek
2018-02-07 20:53 ` Jason Merrill
2018-02-07 22:43 ` Jakub Jelinek
2018-02-07 21:22 ` Jason Merrill
2018-02-08 16:37 ` Jakub Jelinek
2018-02-08 12:30 ` Marek Polacek
2018-02-08 14:56 ` Jason Merrill
2018-02-08 10:15 ` Richard Sandiford
2018-02-08 11:53 ` Marek Polacek [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180208115255.GG2608@redhat.com \
--to=polacek@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=jason@redhat.com \
--cc=nathan@acm.org \
--cc=richard.guenther@gmail.com \
--cc=richard.sandiford@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).