Message ID | Pine.LNX.4.64.1206201635230.29474@wotan.suse.de |
---|---|
State | New |
Headers | show |
On Wed, Jun 20, 2012 at 4:57 PM, Michael Matz <matz@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; > +}
Hi, On Wed, 20 Jun 2012, Richard Guenther wrote: > > + 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. Hmm? The original expression is an address, I have to build a MEM_REF out of that, and the is_gimple_mem_ref_addr() just checked that that very address (after going through fold) is not acceptable as MEM_REF operand. So how could I avoid the massaging of the address to make it an acceptable operand? Ciao, Michael.
On Wed, Jun 20, 2012 at 5:09 PM, Michael Matz <matz@suse.de> wrote: > Hi, > > On Wed, 20 Jun 2012, Richard Guenther wrote: > >> > + 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. > > Hmm? The original expression is an address, I have to build a MEM_REF out > of that, and the is_gimple_mem_ref_addr() just checked that that very > address (after going through fold) is not acceptable as MEM_REF operand. > So how could I avoid the massaging of the address to make it an acceptable > operand? Not change get_base_addres and use if (!is_gimple_mem_ref_addr (TREE_OPERAND (exp, 0)) && (exp = get_base_address (TREE_OPERAND (orig_exp, 0)))) Richard.
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))) { - 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; +}