This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Preserve GCC 5 behavior for PR71002
- From: Richard Biener <rguenther at suse dot de>
- To: gcc-patches at gcc dot gnu dot org
- Cc: Jakub Jelinek <jakub at redhat dot com>
- Date: Tue, 10 May 2016 10:18:59 +0200 (CEST)
- Subject: [PATCH] Preserve GCC 5 behavior for PR71002
- Authentication-results: sourceware.org; auth=none
The following patch fixes optimize_bit_field_compare / fold_truth_andor_1
to preserve TBAA behavior of the original access. This makes us
preserve the implementation behavior of the C family languages using
alias-set zero for all union accesses:
/* Permit type-punning when accessing a union, provided the access
is directly through the union. For example, this code does not
permit taking the address of a union member and then storing
through it. Even the type-punning allowed here is a GCC
extension, albeit a common and useful one; the C standard says
that such accesses have implementation-defined behavior. */
for (u = t;
TREE_CODE (u) == COMPONENT_REF || TREE_CODE (u) == ARRAY_REF;
u = TREE_OPERAND (u, 0))
if (TREE_CODE (u) == COMPONENT_REF
&& TREE_CODE (TREE_TYPE (TREE_OPERAND (u, 0))) == UNION_TYPE)
return 0;
in the PR71002 testcase the issue is that it references the non-active
union member (indirectly through *this, so technically not covered
by our punning rules).
optimize_bit_field_compare turns
((const struct string *) this)->m_repr.s.h.is_short != 0
into
(BIT_FIELD_REF <*(const struct string *) this, 8, 0> & 1) != 0
where the former is a union access using alias-set zero and the latter
not, using the alias set of 'string'.
There are two options - not apply this folding if it doesn't preserve
TBAA behavior (first patch) or preserve TBAA behavior by wrapping
the base into a MEM_REF (which also requires fixing
reference_alias_ptr_type, sth which we need to do anyway).
See the two patches below.
Incidentially the folding (which I still consider premature) prevents
CSE on the GIMPLE level as well:
<bb 2>:
MEM[(struct &)this_2(D)] ={v} {CLOBBER};
_4 = &this_2(D)->m_str;
MEM[(struct &)this_2(D)] ={v} {CLOBBER};
MEM[(struct short_t &)this_2(D)].h.is_short = 1;
MEM[(struct short_t &)this_2(D)].h.length = 0;
MEM[(struct short_t &)this_2(D)].data[0] = 0;
_19 = BIT_FIELD_REF <MEM[(void *)this_2(D)], 8, 0>;
_20 = _19 & 1;
if (_20 != 0)
Here we can't CSE the is_short access and optimize the compare.
So on trunk I am thinking of at least removing the compare
against constant case. [I've removed the whole code twice in
GCC history and it always got installed back ...]
Bootstrap and regtest of both patches running on x86_64-unknown-linux-gnu.
Any preference for GCC 6?
Thanks,
Richard.
2016-05-10 Richard Biener <rguenther@suse.de>
PR middle-end/71002
* fold-const.c (optimize_bit_field_compare): If using a
BIT_FIELD_REF instead of the component ref does not
preserve alias-sets, bail out.
(fold_truth_andor_1): Likewise.
Index: gcc/fold-const.c
===================================================================
*** gcc/fold-const.c (revision 236032)
--- gcc/fold-const.c (working copy)
*************** static enum tree_code compcode_to_compar
*** 117,124 ****
static int operand_equal_for_comparison_p (tree, tree, tree);
static int twoval_comparison_p (tree, tree *, tree *, int *);
static tree eval_subst (location_t, tree, tree, tree, tree, tree);
- static tree make_bit_field_ref (location_t, tree, tree,
- HOST_WIDE_INT, HOST_WIDE_INT, int, int);
static tree optimize_bit_field_compare (location_t, enum tree_code,
tree, tree, tree);
static tree decode_field_reference (location_t, tree, HOST_WIDE_INT *,
--- 117,122 ----
*************** optimize_bit_field_compare (location_t l
*** 3859,3865 ****
linner = get_inner_reference (lhs, &lbitsize, &lbitpos, &offset, &lmode,
&lunsignedp, &lreversep, &lvolatilep, false);
if (linner == lhs || lbitsize == GET_MODE_BITSIZE (lmode) || lbitsize < 0
! || offset != 0 || TREE_CODE (linner) == PLACEHOLDER_EXPR || lvolatilep)
return 0;
if (const_p)
--- 3857,3865 ----
linner = get_inner_reference (lhs, &lbitsize, &lbitpos, &offset, &lmode,
&lunsignedp, &lreversep, &lvolatilep, false);
if (linner == lhs || lbitsize == GET_MODE_BITSIZE (lmode) || lbitsize < 0
! || offset != 0 || TREE_CODE (linner) == PLACEHOLDER_EXPR || lvolatilep
! /* Make sure we can preserve alias-sets. */
! || get_alias_set (lhs) != get_alias_set (linner))
return 0;
if (const_p)
*************** optimize_bit_field_compare (location_t l
*** 3874,3880 ****
if (rinner == rhs || lbitpos != rbitpos || lbitsize != rbitsize
|| lunsignedp != runsignedp || lreversep != rreversep || offset != 0
! || TREE_CODE (rinner) == PLACEHOLDER_EXPR || rvolatilep)
return 0;
}
--- 3874,3882 ----
if (rinner == rhs || lbitpos != rbitpos || lbitsize != rbitsize
|| lunsignedp != runsignedp || lreversep != rreversep || offset != 0
! || TREE_CODE (rinner) == PLACEHOLDER_EXPR || rvolatilep
! /* Make sure we can preserve alias-sets. */
! || get_alias_set (rhs) != get_alias_set (rinner))
return 0;
}
*************** fold_truth_andor_1 (location_t loc, enum
*** 5791,5797 ****
|| ll_unsignedp != lr_unsignedp || rl_unsignedp != rr_unsignedp
/* Make sure the two fields on the right
correspond to the left without being swapped. */
! || ll_bitpos - rl_bitpos != lr_bitpos - rr_bitpos)
return 0;
first_bit = MIN (lr_bitpos, rr_bitpos);
--- 5793,5802 ----
|| ll_unsignedp != lr_unsignedp || rl_unsignedp != rr_unsignedp
/* Make sure the two fields on the right
correspond to the left without being swapped. */
! || ll_bitpos - rl_bitpos != lr_bitpos - rr_bitpos
! /* Make sure we can preserve alias-sets. */
! || get_alias_set (ll_arg) != get_alias_set (ll_inner)
! || get_alias_set (lr_arg) != get_alias_set (lr_inner))
return 0;
first_bit = MIN (lr_bitpos, rr_bitpos);
*************** fold_truth_andor_1 (location_t loc, enum
*** 5921,5926 ****
--- 5926,5935 ----
}
}
+ /* Make sure we can preserve alias-sets. */
+ if (get_alias_set (ll_arg) != get_alias_set (ll_inner))
+ return NULL_TREE;
+
/* Construct the expression we will return. First get the component
reference we will make. Unless the mask is all ones the width of
that field, perform the mask operation. Then compare with the
2016-05-10 Richard Biener <rguenther@suse.de>
PR middle-end/71002
* alias.c (reference_alias_ptr_type): Preserve alias-set zero
if the langhook insists on it.
* fold-const.c (make_bit_field_ref): Add arg for the original
reference and preserve its alias-set.
(optimize_bit_field_compare): Adjust callers.
(fold_truth_andor_1): Likewise.
Index: gcc/fold-const.c
===================================================================
*** gcc/fold-const.c (revision 236032)
--- gcc/fold-const.c (working copy)
*************** static enum tree_code compcode_to_compar
*** 117,124 ****
static int operand_equal_for_comparison_p (tree, tree, tree);
static int twoval_comparison_p (tree, tree *, tree *, int *);
static tree eval_subst (location_t, tree, tree, tree, tree, tree);
- static tree make_bit_field_ref (location_t, tree, tree,
- HOST_WIDE_INT, HOST_WIDE_INT, int, int);
static tree optimize_bit_field_compare (location_t, enum tree_code,
tree, tree, tree);
static tree decode_field_reference (location_t, tree, HOST_WIDE_INT *,
--- 117,122 ----
*************** distribute_real_division (location_t loc
*** 3781,3795 ****
/* Return a BIT_FIELD_REF of type TYPE to refer to BITSIZE bits of INNER
starting at BITPOS. The field is unsigned if UNSIGNEDP is nonzero
! and uses reverse storage order if REVERSEP is nonzero. */
static tree
! make_bit_field_ref (location_t loc, tree inner, tree type,
HOST_WIDE_INT bitsize, HOST_WIDE_INT bitpos,
int unsignedp, int reversep)
{
tree result, bftype;
if (bitpos == 0 && !reversep)
{
tree size = TYPE_SIZE (TREE_TYPE (inner));
--- 3779,3801 ----
/* Return a BIT_FIELD_REF of type TYPE to refer to BITSIZE bits of INNER
starting at BITPOS. The field is unsigned if UNSIGNEDP is nonzero
! and uses reverse storage order if REVERSEP is nonzero. ORIG_INNER
! is the original memory reference used to preserve the alias set of
! the access. */
static tree
! make_bit_field_ref (location_t loc, tree inner, tree orig_inner, tree type,
HOST_WIDE_INT bitsize, HOST_WIDE_INT bitpos,
int unsignedp, int reversep)
{
tree result, bftype;
+ if (get_alias_set (inner) != get_alias_set (orig_inner))
+ inner = fold_build2 (MEM_REF, TREE_TYPE (inner),
+ build_fold_addr_expr (inner),
+ build_int_cst
+ (reference_alias_ptr_type (orig_inner), 0));
+
if (bitpos == 0 && !reversep)
{
tree size = TYPE_SIZE (TREE_TYPE (inner));
*************** optimize_bit_field_compare (location_t l
*** 3915,3927 ****
and return. */
return fold_build2_loc (loc, code, compare_type,
fold_build2_loc (loc, BIT_AND_EXPR, unsigned_type,
! make_bit_field_ref (loc, linner,
unsigned_type,
nbitsize, nbitpos,
1, lreversep),
mask),
fold_build2_loc (loc, BIT_AND_EXPR, unsigned_type,
! make_bit_field_ref (loc, rinner,
unsigned_type,
nbitsize, nbitpos,
1, rreversep),
--- 3921,3933 ----
and return. */
return fold_build2_loc (loc, code, compare_type,
fold_build2_loc (loc, BIT_AND_EXPR, unsigned_type,
! make_bit_field_ref (loc, linner, lhs,
unsigned_type,
nbitsize, nbitpos,
1, lreversep),
mask),
fold_build2_loc (loc, BIT_AND_EXPR, unsigned_type,
! make_bit_field_ref (loc, rinner, rhs,
unsigned_type,
nbitsize, nbitpos,
1, rreversep),
*************** optimize_bit_field_compare (location_t l
*** 3966,3973 ****
/* Make a new bitfield reference, shift the constant over the
appropriate number of bits and mask it with the computed mask
(in case this was a signed field). If we changed it, make a new one. */
! lhs = make_bit_field_ref (loc, linner, unsigned_type, nbitsize, nbitpos, 1,
! lreversep);
rhs = const_binop (BIT_AND_EXPR,
const_binop (LSHIFT_EXPR,
--- 3972,3979 ----
/* Make a new bitfield reference, shift the constant over the
appropriate number of bits and mask it with the computed mask
(in case this was a signed field). If we changed it, make a new one. */
! lhs = make_bit_field_ref (loc, linner, lhs, unsigned_type,
! nbitsize, nbitpos, 1, lreversep);
rhs = const_binop (BIT_AND_EXPR,
const_binop (LSHIFT_EXPR,
*************** fold_truth_andor_1 (location_t loc, enum
*** 5829,5840 ****
lr_mask = const_binop (BIT_IOR_EXPR, lr_mask, rr_mask);
if (lnbitsize == rnbitsize && xll_bitpos == xlr_bitpos)
{
! lhs = make_bit_field_ref (loc, ll_inner, lntype, lnbitsize, lnbitpos,
ll_unsignedp || rl_unsignedp, ll_reversep);
if (! all_ones_mask_p (ll_mask, lnbitsize))
lhs = build2 (BIT_AND_EXPR, lntype, lhs, ll_mask);
! rhs = make_bit_field_ref (loc, lr_inner, rntype, rnbitsize, rnbitpos,
lr_unsignedp || rr_unsignedp, lr_reversep);
if (! all_ones_mask_p (lr_mask, rnbitsize))
rhs = build2 (BIT_AND_EXPR, rntype, rhs, lr_mask);
--- 5835,5848 ----
lr_mask = const_binop (BIT_IOR_EXPR, lr_mask, rr_mask);
if (lnbitsize == rnbitsize && xll_bitpos == xlr_bitpos)
{
! lhs = make_bit_field_ref (loc, ll_inner, ll_arg,
! lntype, lnbitsize, lnbitpos,
ll_unsignedp || rl_unsignedp, ll_reversep);
if (! all_ones_mask_p (ll_mask, lnbitsize))
lhs = build2 (BIT_AND_EXPR, lntype, lhs, ll_mask);
! rhs = make_bit_field_ref (loc, lr_inner, lr_arg,
! rntype, rnbitsize, rnbitpos,
lr_unsignedp || rr_unsignedp, lr_reversep);
if (! all_ones_mask_p (lr_mask, rnbitsize))
rhs = build2 (BIT_AND_EXPR, rntype, rhs, lr_mask);
*************** fold_truth_andor_1 (location_t loc, enum
*** 5856,5866 ****
{
tree type;
! lhs = make_bit_field_ref (loc, ll_inner, lntype,
ll_bitsize + rl_bitsize,
MIN (ll_bitpos, rl_bitpos),
ll_unsignedp, ll_reversep);
! rhs = make_bit_field_ref (loc, lr_inner, rntype,
lr_bitsize + rr_bitsize,
MIN (lr_bitpos, rr_bitpos),
lr_unsignedp, lr_reversep);
--- 5864,5874 ----
{
tree type;
! lhs = make_bit_field_ref (loc, ll_inner, ll_arg, lntype,
ll_bitsize + rl_bitsize,
MIN (ll_bitpos, rl_bitpos),
ll_unsignedp, ll_reversep);
! rhs = make_bit_field_ref (loc, lr_inner, lr_arg, rntype,
lr_bitsize + rr_bitsize,
MIN (lr_bitpos, rr_bitpos),
lr_unsignedp, lr_reversep);
*************** fold_truth_andor_1 (location_t loc, enum
*** 5925,5931 ****
reference we will make. Unless the mask is all ones the width of
that field, perform the mask operation. Then compare with the
merged constant. */
! result = make_bit_field_ref (loc, ll_inner, lntype, lnbitsize, lnbitpos,
ll_unsignedp || rl_unsignedp, ll_reversep);
ll_mask = const_binop (BIT_IOR_EXPR, ll_mask, rl_mask);
--- 5933,5940 ----
reference we will make. Unless the mask is all ones the width of
that field, perform the mask operation. Then compare with the
merged constant. */
! result = make_bit_field_ref (loc, ll_inner, ll_arg,
! lntype, lnbitsize, lnbitpos,
ll_unsignedp || rl_unsignedp, ll_reversep);
ll_mask = const_binop (BIT_IOR_EXPR, ll_mask, rl_mask);
Index: gcc/alias.c
===================================================================
*** gcc/alias.c (revision 236032)
--- gcc/alias.c (working copy)
*************** reference_alias_ptr_type_1 (tree *t)
*** 769,774 ****
--- 769,778 ----
tree
reference_alias_ptr_type (tree t)
{
+ /* If the frontend assigns this alias-set zero, preserve that. */
+ if (lang_hooks.get_alias_set (t) == 0)
+ return ptr_type_node;
+
tree ptype = reference_alias_ptr_type_1 (&t);
/* If there is a given pointer type for aliasing purposes, return it. */
if (ptype != NULL_TREE)