When a bitwise OR gimple statement has for operands SSA_NAME initialized directly from memory source (no cast or other unary statement intervening), a symbolic number will be used only partly initialized. This was discovered by valgrind and reported as PR61328. This patch fixes that by moving the initialization code in a separate function that can be called from the two places that need it. There was a problem of a field of a structure that was set in a function and the value of this field was read before checking the result of the function call. This would lead to missed optimization.
ChangeLog is as follows:
2014-05-29 Thomas Preud'homme <thomas.preudho...@arm.com>
PR tree-optimization/61328
* tree-ssa-math-opts.c (init_symbolic_number): Extract symbolic number
initialization from find_bswap_or_nop_1.
(find_bswap_or_nop_1): Test return value of find_bswap_or_nop_1 stored
in source_expr2 before using the size value the function sets. Also
make use of init_symbolic_number () in both the old place and
find_bswap_or_nop_load () to avoid reading uninitialized memory when
doing recursion in the GIMPLE_BINARY_RHS case.
Ok for trunk?
diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
index d9afccf..6c26d6d 100644
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -1701,6 +1701,30 @@ verify_symbolic_number_p (struct symbolic_number *n,
gimple stmt)
return true;
}
+/* Initialize the symbolic number N for the bswap pass from the base element
+ SRC manipulated by the bitwise OR expression. */
+
+static bool
+init_symbolic_number (struct symbolic_number *n, tree src)
+{
+ n->base_addr = n->offset = n->alias_set = n->vuse = NULL_TREE;
+
+ /* Set up the symbolic number N by setting each byte to a value between 1 and
+ the byte size of rhs1. The highest order byte is set to n->size and the
+ lowest order byte to 1. */
+ n->size = TYPE_PRECISION (TREE_TYPE (src));
+ if (n->size % BITS_PER_UNIT != 0)
+ return false;
+ n->size /= BITS_PER_UNIT;
+ n->range = n->size;
+ n->n = CMPNOP;
+
+ if (n->size < (int)sizeof (int64_t))
+ n->n &= ((uint64_t)1 << (n->size * BITS_PER_UNIT)) - 1;
+
+ return true;
+}
+
/* Check if STMT might be a byte swap or a nop from a memory source and returns
the answer. If so, REF is that memory source and the base of the memory area
accessed and the offset of the access from that base are recorded in N. */
@@ -1713,26 +1737,27 @@ find_bswap_or_nop_load (gimple stmt, tree ref, struct
symbolic_number *n)
HOST_WIDE_INT bitsize, bitpos;
enum machine_mode mode;
int unsignedp, volatilep;
+ tree offset, base_addr;
if (!gimple_assign_load_p (stmt) || gimple_has_volatile_ops (stmt))
return false;
- n->base_addr = get_inner_reference (ref, &bitsize, &bitpos, &n->offset,
- &mode, &unsignedp, &volatilep, false);
+ base_addr = get_inner_reference (ref, &bitsize, &bitpos, &offset, &mode,
+ &unsignedp, &volatilep, false);
- if (TREE_CODE (n->base_addr) == MEM_REF)
+ if (TREE_CODE (base_addr) == MEM_REF)
{
offset_int bit_offset = 0;
- tree off = TREE_OPERAND (n->base_addr, 1);
+ tree off = TREE_OPERAND (base_addr, 1);
if (!integer_zerop (off))
{
- offset_int boff, coff = mem_ref_offset (n->base_addr);
+ offset_int boff, coff = mem_ref_offset (base_addr);
boff = wi::lshift (coff, LOG2_BITS_PER_UNIT);
bit_offset += boff;
}
- n->base_addr = TREE_OPERAND (n->base_addr, 0);
+ base_addr = TREE_OPERAND (base_addr, 0);
/* Avoid returning a negative bitpos as this may wreak havoc later. */
if (wi::neg_p (bit_offset))
@@ -1743,11 +1768,11 @@ find_bswap_or_nop_load (gimple stmt, tree ref, struct
symbolic_number *n)
Subtract it to BIT_OFFSET and add it (scaled) to OFFSET. */
bit_offset -= tem;
tem = wi::arshift (tem, LOG2_BITS_PER_UNIT);
- if (n->offset)
- n->offset = size_binop (PLUS_EXPR, n->offset,
+ if (offset)
+ offset = size_binop (PLUS_EXPR, offset,
wide_int_to_tree (sizetype, tem));
else
- n->offset = wide_int_to_tree (sizetype, tem);
+ offset = wide_int_to_tree (sizetype, tem);
}
bitpos += bit_offset.to_shwi ();
@@ -1758,6 +1783,9 @@ find_bswap_or_nop_load (gimple stmt, tree ref, struct
symbolic_number *n)
if (bitsize % BITS_PER_UNIT)
return false;
+ init_symbolic_number (n, ref);
+ n->base_addr = base_addr;
+ n->offset = offset;
n->bytepos = bitpos / BITS_PER_UNIT;
n->alias_set = reference_alias_ptr_type (ref);
n->vuse = gimple_vuse (stmt);
@@ -1816,28 +1844,12 @@ find_bswap_or_nop_1 (gimple stmt, struct
symbolic_number *n, int limit)
/* If find_bswap_or_nop_1 returned NULL, STMT is a leaf node and
we have to initialize the symbolic number. */
- if (!source_expr1 || gimple_assign_load_p (rhs1_stmt))
+ if (!source_expr1)
{
- /* Set up the symbolic number N by setting each byte to a
- value between 1 and the byte size of rhs1. The highest
- order byte is set to n->size and the lowest order
- byte to 1. */
- n->size = TYPE_PRECISION (TREE_TYPE (rhs1));
- if (n->size % BITS_PER_UNIT != 0)
+ if (gimple_assign_load_p (stmt)
+ || !init_symbolic_number (n, rhs1))
return NULL_TREE;
- n->size /= BITS_PER_UNIT;
- n->range = n->size;
- n->n = CMPNOP;
-
- if (n->size < (int)sizeof (int64_t))
- n->n &= ((uint64_t)1 <<
- (n->size * BITS_PER_UNIT)) - 1;
-
- if (!source_expr1)
- {
- n->base_addr = n->offset = n->alias_set = n->vuse = NULL_TREE;
- source_expr1 = rhs1;
- }
+ source_expr1 = rhs1;
}
switch (code)
@@ -1915,7 +1927,10 @@ find_bswap_or_nop_1 (gimple stmt, struct symbolic_number
*n, int limit)
source_expr2 = find_bswap_or_nop_1 (rhs2_stmt, &n2, limit - 1);
- if (n1.size != n2.size || !source_expr2)
+ if (!source_expr2)
+ return NULL_TREE;
+
+ if (n1.size != n2.size)
return NULL_TREE;
if (!n1.vuse != !n2.vuse ||
Best regards,
Thomas
PR61328.1.1.diff
Description: Binary data
