On Tue, 10 May 2016, Jakub Jelinek wrote:
> On Tue, May 10, 2016 at 10:18:59AM +0200, Richard Biener wrote:
> > 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?
>
> I think I prefer the second patch for the branch.
On trunk this exposed an issue with the gimplify-into-SSA patch and
I catched an issue with passing the original ref.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk
sofar.
Richard.
2016-05-11 Richard Biener <[email protected]>
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.
(decode_field_reference): Take exp by reference and adjust it
to the original memory reference.
(optimize_bit_field_compare): Adjust callers.
(fold_truth_andor_1): Likewise.
* gimplify.c (gimplify_expr): Adjust in-SSA form test.
* g++.dg/torture/pr71002.C: New testcase.
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)
Index: gcc/fold-const.c
===================================================================
*** gcc/fold-const.c (revision 236069)
--- gcc/fold-const.c (working copy)
*************** static enum tree_code compcode_to_compar
*** 117,130 ****
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 *,
- HOST_WIDE_INT *,
- machine_mode *, int *, int *, int *,
- tree *, tree *);
static int simple_operand_p (const_tree);
static bool simple_operand_p_2 (tree);
static tree range_binop (enum tree_code, tree, tree, int, tree, int);
--- 117,124 ----
*************** distribute_real_division (location_t loc
*** 3803,3817 ****
/* 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));
--- 3797,3819 ----
/* 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
*** 3937,3949 ****
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),
--- 3939,3951 ----
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
*** 3988,3995 ****
/* 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,
--- 3990,3997 ----
/* 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,
*************** optimize_bit_field_compare (location_t l
*** 4028,4038 ****
do anything with. */
static tree
! decode_field_reference (location_t loc, tree exp, HOST_WIDE_INT *pbitsize,
HOST_WIDE_INT *pbitpos, machine_mode *pmode,
int *punsignedp, int *preversep, int *pvolatilep,
tree *pmask, tree *pand_mask)
{
tree outer_type = 0;
tree and_mask = 0;
tree mask, inner, offset;
--- 4030,4041 ----
do anything with. */
static tree
! decode_field_reference (location_t loc, tree *exp_, HOST_WIDE_INT *pbitsize,
HOST_WIDE_INT *pbitpos, machine_mode *pmode,
int *punsignedp, int *preversep, int *pvolatilep,
tree *pmask, tree *pand_mask)
{
+ tree exp = *exp_;
tree outer_type = 0;
tree and_mask = 0;
tree mask, inner, offset;
*************** decode_field_reference (location_t loc,
*** 4069,4074 ****
--- 4072,4079 ----
|| TREE_CODE (inner) == PLACEHOLDER_EXPR)
return 0;
+ *exp_ = exp;
+
/* If the number of bits in the reference is the same as the bitsize of
the outer type, then the outer type gives the signedness. Otherwise
(in case of a small bitfield) the signedness is unchanged. */
*************** fold_truth_andor_1 (location_t loc, enum
*** 5677,5695 ****
ll_reversep = lr_reversep = rl_reversep = rr_reversep = 0;
volatilep = 0;
! ll_inner = decode_field_reference (loc, ll_arg,
&ll_bitsize, &ll_bitpos, &ll_mode,
&ll_unsignedp, &ll_reversep, &volatilep,
&ll_mask, &ll_and_mask);
! lr_inner = decode_field_reference (loc, lr_arg,
&lr_bitsize, &lr_bitpos, &lr_mode,
&lr_unsignedp, &lr_reversep, &volatilep,
&lr_mask, &lr_and_mask);
! rl_inner = decode_field_reference (loc, rl_arg,
&rl_bitsize, &rl_bitpos, &rl_mode,
&rl_unsignedp, &rl_reversep, &volatilep,
&rl_mask, &rl_and_mask);
! rr_inner = decode_field_reference (loc, rr_arg,
&rr_bitsize, &rr_bitpos, &rr_mode,
&rr_unsignedp, &rr_reversep, &volatilep,
&rr_mask, &rr_and_mask);
--- 5682,5700 ----
ll_reversep = lr_reversep = rl_reversep = rr_reversep = 0;
volatilep = 0;
! ll_inner = decode_field_reference (loc, &ll_arg,
&ll_bitsize, &ll_bitpos, &ll_mode,
&ll_unsignedp, &ll_reversep, &volatilep,
&ll_mask, &ll_and_mask);
! lr_inner = decode_field_reference (loc, &lr_arg,
&lr_bitsize, &lr_bitpos, &lr_mode,
&lr_unsignedp, &lr_reversep, &volatilep,
&lr_mask, &lr_and_mask);
! rl_inner = decode_field_reference (loc, &rl_arg,
&rl_bitsize, &rl_bitpos, &rl_mode,
&rl_unsignedp, &rl_reversep, &volatilep,
&rl_mask, &rl_and_mask);
! rr_inner = decode_field_reference (loc, &rr_arg,
&rr_bitsize, &rr_bitpos, &rr_mode,
&rr_unsignedp, &rr_reversep, &volatilep,
&rr_mask, &rr_and_mask);
*************** fold_truth_andor_1 (location_t loc, enum
*** 5851,5862 ****
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);
--- 5856,5869 ----
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
*** 5878,5888 ****
{
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);
--- 5885,5895 ----
{
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
*** 5947,5953 ****
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);
--- 5954,5961 ----
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/gimplify.c
===================================================================
*** gcc/gimplify.c (revision 236077)
--- gcc/gimplify.c (working copy)
*************** gimplify_expr (tree *expr_p, gimple_seq
*** 10452,10458 ****
in suitable form. Re-gimplifying would mark the address
operand addressable. Always gimplify when not in SSA form
as we still may have to gimplify decls with value-exprs. */
! if (!gimplify_ctxp || !gimplify_ctxp->into_ssa
|| !is_gimple_mem_ref_addr (TREE_OPERAND (*expr_p, 0)))
{
ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
--- 10452,10458 ----
in suitable form. Re-gimplifying would mark the address
operand addressable. Always gimplify when not in SSA form
as we still may have to gimplify decls with value-exprs. */
! if (!gimplify_ctxp || !gimple_in_ssa_p (cfun)
|| !is_gimple_mem_ref_addr (TREE_OPERAND (*expr_p, 0)))
{
ret = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
Index: gcc/testsuite/g++.dg/torture/pr71002.C
===================================================================
*** gcc/testsuite/g++.dg/torture/pr71002.C (revision 0)
--- gcc/testsuite/g++.dg/torture/pr71002.C (revision 0)
***************
*** 0 ****
--- 1,160 ----
+ // { dg-do run }
+
+ using size_t = __SIZE_TYPE__;
+
+ inline void* operator new(size_t, void* p) noexcept
+ { return p; }
+
+ inline void operator delete(void*, void*)
+ { }
+
+ struct long_t
+ {
+ size_t is_short : 1;
+ size_t length : (__SIZEOF_SIZE_T__ * __CHAR_BIT__ - 1);
+ size_t capacity;
+ char* pointer;
+ };
+
+ union long_raw_t {
+ unsigned char data[sizeof(long_t)];
+ struct __attribute__((aligned(alignof(long_t)))) { } align;
+ };
+
+ struct short_header
+ {
+ unsigned char is_short : 1;
+ unsigned char length : (__CHAR_BIT__ - 1);
+ };
+
+ struct short_t
+ {
+ short_header h;
+ char data[23];
+ };
+
+ union repr_t
+ {
+ long_raw_t r;
+ short_t s;
+
+ const short_t& short_repr() const
+ { return s; }
+
+ const long_t& long_repr() const
+ { return *static_cast<const long_t*>(static_cast<const void*>(&r)); }
+
+ short_t& short_repr()
+ { return s; }
+
+ long_t& long_repr()
+ { return *static_cast<long_t*>(static_cast<void*>(&r)); }
+ };
+
+ class string
+ {
+ public:
+ string()
+ {
+ short_t& s = m_repr.short_repr();
+ s.h.is_short = 1;
+ s.h.length = 0;
+ s.data[0] = '\0';
+ }
+
+ string(const char* str)
+ {
+ size_t length = __builtin_strlen(str);
+ if (length + 1 > 23) {
+ long_t& l = m_repr.long_repr();
+ l.is_short = 0;
+ l.length = length;
+ l.capacity = length + 1;
+ l.pointer = new char[l.capacity];
+ __builtin_memcpy(l.pointer, str, length + 1);
+ } else {
+ short_t& s = m_repr.short_repr();
+ s.h.is_short = 1;
+ s.h.length = length;
+ __builtin_memcpy(s.data, str, length + 1);
+ }
+ }
+
+ string(string&& other)
+ : string{}
+ {
+ swap_data(other);
+ }
+
+ ~string()
+ {
+ if (!is_short()) {
+ delete[] m_repr.long_repr().pointer;
+ }
+ }
+
+ size_t length() const
+ { return is_short() ? short_length() : long_length(); }
+
+ private:
+ bool is_short() const
+ { return m_repr.s.h.is_short != 0; }
+
+ size_t short_length() const
+ { return m_repr.short_repr().h.length; }
+
+ size_t long_length() const
+ { return m_repr.long_repr().length; }
+
+ void swap_data(string& other)
+ {
+ if (is_short()) {
+ if (other.is_short()) {
+ repr_t tmp(m_repr);
+ m_repr = other.m_repr;
+ other.m_repr = tmp;
+ } else {
+ short_t short_backup(m_repr.short_repr());
+ m_repr.short_repr().~short_t();
+ ::new(&m_repr.long_repr()) long_t(other.m_repr.long_repr());
+ other.m_repr.long_repr().~long_t();
+ ::new(&other.m_repr.short_repr()) short_t(short_backup);
+ }
+ } else {
+ if (other.is_short()) {
+ short_t short_backup(other.m_repr.short_repr());
+ other.m_repr.short_repr().~short_t();
+ ::new(&other.m_repr.long_repr()) long_t(m_repr.long_repr());
+ m_repr.long_repr().~long_t();
+ ::new(&m_repr.short_repr()) short_t(short_backup);
+ } else {
+ long_t tmp(m_repr.long_repr());
+ m_repr.long_repr() = other.m_repr.long_repr();
+ other.m_repr.long_repr() = tmp;
+ }
+ }
+ }
+
+ repr_t m_repr;
+ };
+
+ struct foo
+ {
+ __attribute__((noinline))
+ foo(string str)
+ : m_str{static_cast<string&&>(str)},
+ m_len{m_str.length()}
+ { }
+
+ string m_str;
+ size_t m_len;
+ };
+
+ int main()
+ {
+ foo f{"the quick brown fox jumps over the lazy dog"};
+ if (f.m_len == 0) {
+ __builtin_abort();
+ }
+ return 0;
+ }
