On Wed, Jun 20, 2012 at 4:57 PM, Michael Matz <m...@suse.de> wrote:
> Hi,
>
> On Tue, 19 Jun 2012, Richard Guenther wrote:
>
>> The MEM_REF is acceptable to the tree oracle and it can extract
>> points-to information from it.
>>
>> Thus for simplicity unconditionally building the above is the best.
>
> But it doesn't work, as refs_may_alias_p_1 only accepts certain operands
> in MEM_REFs. So, I opted to check the operand for is_gimple_mem_ref_addr
> after it's built, and if not acceptable at least build a mem-ref for the
> base object, if possible. In order not to loose info we had before the
> patch I had to improve get_base_address a little to not give up on
> MEM_REFs like "MEM[&p.c]".
>
> Regstrapped on x86_64-linux, no regressions. Okay for trunk?
>
Hrm ...
> Ciao,
> Michael.
> PR middle-end/53688
> * gimple.c (get_base_address): Strip components also from inner
> arguments to MEM_REFs.
> * builtins.c (get_memory_rtx): Always build an all-aliasing MEM_REF
> with correct size.
>
> testsuite/
> * gcc.c-torture/execute/pr53688.c: New test.
> Index: gimple.c
> ===================================================================
> --- gimple.c (revision 188772)
> +++ gimple.c (working copy)
> @@ -2911,7 +2911,11 @@ get_base_address (tree t)
> if ((TREE_CODE (t) == MEM_REF
> || TREE_CODE (t) == TARGET_MEM_REF)
> && TREE_CODE (TREE_OPERAND (t, 0)) == ADDR_EXPR)
> - t = TREE_OPERAND (TREE_OPERAND (t, 0), 0);
> + {
> + t = TREE_OPERAND (TREE_OPERAND (t, 0), 0);
> + while (handled_component_p (t))
> + t = TREE_OPERAND (t, 0);
> + }
>
> if (TREE_CODE (t) == SSA_NAME
> || DECL_P (t)
> Index: builtins.c
> ===================================================================
> --- builtins.c (revision 188772)
> +++ builtins.c (working copy)
> @@ -1252,7 +1252,6 @@ get_memory_rtx (tree exp, tree len)
> {
> tree orig_exp = exp;
> rtx addr, mem;
> - HOST_WIDE_INT off;
>
> /* When EXP is not resolved SAVE_EXPR, MEM_ATTRS can be still derived
> from its expression, for expr->a.b only <variable>.a.b is recorded. */
> @@ -1269,114 +1268,30 @@ get_memory_rtx (tree exp, tree len)
> && POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (exp, 0))))
> exp = TREE_OPERAND (exp, 0);
>
> - off = 0;
> - if (TREE_CODE (exp) == POINTER_PLUS_EXPR
> - && TREE_CODE (TREE_OPERAND (exp, 0)) == ADDR_EXPR
> - && host_integerp (TREE_OPERAND (exp, 1), 0)
> - && (off = tree_low_cst (TREE_OPERAND (exp, 1), 0)) > 0)
> - exp = TREE_OPERAND (TREE_OPERAND (exp, 0), 0);
> - else if (TREE_CODE (exp) == ADDR_EXPR)
> - exp = TREE_OPERAND (exp, 0);
> - else if (POINTER_TYPE_P (TREE_TYPE (exp)))
> - exp = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (exp)), exp);
> - else
> - exp = NULL;
> -
> - /* Honor attributes derived from exp, except for the alias set
> - (as builtin stringops may alias with anything) and the size
> - (as stringops may access multiple array elements). */
> - if (exp)
> + /* Build a MEM_REF representing the whole accessed area as a byte blob,
> + (as builtin stringops may alias with anything). */
> + exp = fold_build2 (MEM_REF,
> + build_array_type (char_type_node,
> + build_range_type (sizetype,
> + size_one_node, len)),
> + exp, build_int_cst (ptr_type_node, 0));
> +
> + /* If the MEM_REF has no acceptable address, try to get the base object,
> + and build an all-aliasing unknown-sized access to that one. */
> + if (!is_gimple_mem_ref_addr (TREE_OPERAND (exp, 0))
> + && (exp = get_base_address (exp)))
The get_base_address massaging should be not necessary if you'd
use the original exp here, not the built MEM_REF.
Otherwise looks ok.
Thanks,
Richard.
> {
> - set_mem_attributes (mem, exp, 0);
> -
> - if (off)
> - mem = adjust_automodify_address_nv (mem, BLKmode, NULL, off);
> -
> - /* Allow the string and memory builtins to overflow from one
> - field into another, see http://gcc.gnu.org/PR23561.
> - Thus avoid COMPONENT_REFs in MEM_EXPR unless we know the whole
> - memory accessed by the string or memory builtin will fit
> - within the field. */
> - if (MEM_EXPR (mem) && TREE_CODE (MEM_EXPR (mem)) == COMPONENT_REF)
> - {
> - tree mem_expr = MEM_EXPR (mem);
> - HOST_WIDE_INT offset = -1, length = -1;
> - tree inner = exp;
> -
> - while (TREE_CODE (inner) == ARRAY_REF
> - || CONVERT_EXPR_P (inner)
> - || TREE_CODE (inner) == VIEW_CONVERT_EXPR
> - || TREE_CODE (inner) == SAVE_EXPR)
> - inner = TREE_OPERAND (inner, 0);
> -
> - gcc_assert (TREE_CODE (inner) == COMPONENT_REF);
> -
> - if (MEM_OFFSET_KNOWN_P (mem))
> - offset = MEM_OFFSET (mem);
> -
> - if (offset >= 0 && len && host_integerp (len, 0))
> - length = tree_low_cst (len, 0);
> -
> - while (TREE_CODE (inner) == COMPONENT_REF)
> - {
> - tree field = TREE_OPERAND (inner, 1);
> - gcc_assert (TREE_CODE (mem_expr) == COMPONENT_REF);
> - gcc_assert (field == TREE_OPERAND (mem_expr, 1));
> -
> - /* Bitfields are generally not byte-addressable. */
> - gcc_assert (!DECL_BIT_FIELD (field)
> - || ((tree_low_cst (DECL_FIELD_BIT_OFFSET (field), 1)
> - % BITS_PER_UNIT) == 0
> - && host_integerp (DECL_SIZE (field), 0)
> - && (TREE_INT_CST_LOW (DECL_SIZE (field))
> - % BITS_PER_UNIT) == 0));
> -
> - /* If we can prove that the memory starting at XEXP (mem, 0) and
> - ending at XEXP (mem, 0) + LENGTH will fit into this field, we
> - can keep the COMPONENT_REF in MEM_EXPR. But be careful with
> - fields without DECL_SIZE_UNIT like flexible array members.
> */
> - if (length >= 0
> - && DECL_SIZE_UNIT (field)
> - && host_integerp (DECL_SIZE_UNIT (field), 0))
> - {
> - HOST_WIDE_INT size
> - = TREE_INT_CST_LOW (DECL_SIZE_UNIT (field));
> - if (offset <= size
> - && length <= size
> - && offset + length <= size)
> - break;
> - }
> -
> - if (offset >= 0
> - && host_integerp (DECL_FIELD_OFFSET (field), 0))
> - offset += TREE_INT_CST_LOW (DECL_FIELD_OFFSET (field))
> - + tree_low_cst (DECL_FIELD_BIT_OFFSET (field), 1)
> - / BITS_PER_UNIT;
> - else
> - {
> - offset = -1;
> - length = -1;
> - }
> -
> - mem_expr = TREE_OPERAND (mem_expr, 0);
> - inner = TREE_OPERAND (inner, 0);
> - }
> -
> - if (mem_expr == NULL)
> - offset = -1;
> - if (mem_expr != MEM_EXPR (mem))
> - {
> - set_mem_expr (mem, mem_expr);
> - if (offset >= 0)
> - set_mem_offset (mem, offset);
> - else
> - clear_mem_offset (mem);
> - }
> - }
> - set_mem_alias_set (mem, 0);
> - clear_mem_size (mem);
> + exp = build_fold_addr_expr (exp);
> + exp = fold_build2 (MEM_REF,
> + build_array_type (char_type_node,
> + build_range_type (sizetype,
> + size_zero_node,
> + NULL)),
> + exp, build_int_cst (ptr_type_node, 0));
> }
> -
> + if (exp)
> + set_mem_attributes (mem, exp, 0);
> + set_mem_alias_set (mem, 0);
> return mem;
> }
>
> Index: testsuite/gcc.c-torture/execute/pr53688.c
> ===================================================================
> --- testsuite/gcc.c-torture/execute/pr53688.c (revision 0)
> +++ testsuite/gcc.c-torture/execute/pr53688.c (revision 0)
> @@ -0,0 +1,32 @@
> +char headline[256];
> +struct hdr {
> + char part1[9];
> + char part2[8];
> +} p;
> +
> +void __attribute__((noinline,noclone))
> +init()
> +{
> + __builtin_memcpy (p.part1, "FOOBARFOO", sizeof (p.part1));
> + __builtin_memcpy (p.part2, "SPEC CPU", sizeof (p.part2));
> +}
> +
> +int main()
> +{
> + char *x;
> + int c;
> + init();
> + __builtin_memcpy (&headline[0], p.part1, 9);
> + c = 9;
> + x = &headline[0];
> + x = x + c;
> + __builtin_memset (x, ' ', 245);
> + __builtin_memcpy (&headline[10], p.part2, 8);
> + c = 18;
> + x = &headline[0];
> + x = x + c;
> + __builtin_memset (x, ' ', 238);
> + if (headline[10] != 'S')
> + __builtin_abort ();
> + return 0;
> +}
