* [PATCH 0/2] Fix PR 44972 @ 2010-09-08 17:16 Martin Jambor 2010-09-08 17:16 ` [PATCH 1/2] Remove the IPA-SRA call to build_ref_for_offset Martin Jambor 2010-09-08 17:19 ` [PATCH 2/2] Reimplementation of build_ref_for_offset Martin Jambor 0 siblings, 2 replies; 11+ messages in thread From: Martin Jambor @ 2010-09-08 17:16 UTC (permalink / raw) To: GCC Patches; +Cc: Richard Guenther Hi, the following two patches fix PR 44972 which was caused by transition to MEM_REF. The problem is that MEM_REFs can also act like VIEW_CONVERT_EXPRs and before MEM_REF we would have disqualified many (SRA) accesses with them. When we don't, build_ref_for_offset can find itself unable to create a reference consisting of component_refs and array_refs in a situation when we cannot handle it and ICE. The solution is to make build_ref_for_offset create a MEM_REF which is what both patches do. The first one does this for IPA-SRA, the second one for everyone else (intra-SRA and IPA-CP/inlining). The implementations are very similar but I decided to keep them separate because the IPA-SRA can handle more cases (it may add pointer dereferences) and, more importantly, it relies on force_gimple_operand_gsi (which is already used anyway) which can somehow cope with functions that are not gimple_in_ssa_p. The other users don't need either and I did not want to use force_gimple_operand_gsi there when it is not needed (and I wanted to have things a bit more under control because of an extra layer handling bit-fields). More comments in the two messages. I also intend to try and relax some other checks now that we have MEM_REF but I'd like to do that incrementally (if possible at all). I have bootstrapped and tested both patches on x86_64-linux without any problems. I hope you'll find this approach more-or-less acceptable because it really took me frustratingly long to get here. Thanks, Martin ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] Remove the IPA-SRA call to build_ref_for_offset 2010-09-08 17:16 [PATCH 0/2] Fix PR 44972 Martin Jambor @ 2010-09-08 17:16 ` Martin Jambor 2010-09-09 9:01 ` Richard Guenther 2010-09-11 2:09 ` H.J. Lu 2010-09-08 17:19 ` [PATCH 2/2] Reimplementation of build_ref_for_offset Martin Jambor 1 sibling, 2 replies; 11+ messages in thread From: Martin Jambor @ 2010-09-08 17:16 UTC (permalink / raw) To: GCC Patches; +Cc: Richard Guenther [-- Attachment #1: no_bro_in_ipaprop.diff --] [-- Type: text/plain, Size: 5421 bytes --] Hi, this is the same patch I sent here in August: http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00609.html It reimplements parameter alterations in callers of a function being modified by IPA-SRA so that it always produces MEM_REF relying on force_gimple_operand_gsi to break up refs if necessary (e.g. if we have a mem_ref of an addr_expr of an array_ref with variable index of another mem_ref etc...). I have bootstrapped and tested this patch separately on x86_64-linux. OK for trunk? Thanks, Martin 2010-08-08 Martin Jambor <mjambor@suse.cz> PR tree-optimization/44972 * ipa-prop.c (ipa_modify_call_arguments): Build MEM_REF instead of calling build_ref_for_offset. * testsuite/g++.dg/torture/pr34850.C: Remove expected warning. Index: mine/gcc/ipa-prop.c =================================================================== --- mine.orig/gcc/ipa-prop.c +++ mine/gcc/ipa-prop.c @@ -2153,40 +2153,77 @@ ipa_modify_call_arguments (struct cgraph } else if (!adj->remove_param) { - tree expr, orig_expr; - bool allow_ptr, repl_found; + tree expr, base, off; + location_t loc; - orig_expr = expr = gimple_call_arg (stmt, adj->base_index); - if (TREE_CODE (expr) == ADDR_EXPR) - { - allow_ptr = false; - expr = TREE_OPERAND (expr, 0); - } - else - allow_ptr = true; + /* We create a new parameter out of the value of the old one, we can + do the following kind of transformations: - repl_found = build_ref_for_offset (&expr, TREE_TYPE (expr), - adj->offset, adj->type, - allow_ptr); - if (repl_found) - { - if (adj->by_ref) - expr = build_fold_addr_expr (expr); - } + - A scalar passed by reference is converted to a scalar passed by + value. (adj->by_ref is false and the type of the original + actual argument is a pointer to a scalar). + + - A part of an aggregate is passed instead of the whole aggregate. + The part can be passed either by value or by reference, this is + determined by value of adj->by_ref. Moreover, the code below + handles both situations when the original aggregate is passed by + value (its type is not a pointer) and when it is passed by + reference (it is a pointer to an aggregate). + + When the new argument is passed by reference (adj->by_ref is true) + it must be a part of an aggregate and therefore we form it by + simply taking the address of a reference inside the original + aggregate. */ + + gcc_checking_assert (adj->offset % BITS_PER_UNIT == 0); + base = gimple_call_arg (stmt, adj->base_index); + loc = EXPR_LOCATION (base); + + if (TREE_CODE (base) == ADDR_EXPR + && DECL_P (TREE_OPERAND (base, 0))) + off = build_int_cst (reference_alias_ptr_type (base), + adj->offset / BITS_PER_UNIT); + else if (TREE_CODE (base) != ADDR_EXPR + && POINTER_TYPE_P (TREE_TYPE (base))) + off = build_int_cst (TREE_TYPE (base), adj->offset / BITS_PER_UNIT); else { - tree ptrtype = build_pointer_type (adj->type); - expr = orig_expr; - if (!POINTER_TYPE_P (TREE_TYPE (expr))) - expr = build_fold_addr_expr (expr); - if (!useless_type_conversion_p (ptrtype, TREE_TYPE (expr))) - expr = fold_convert (ptrtype, expr); - expr = fold_build2 (POINTER_PLUS_EXPR, ptrtype, expr, - build_int_cst (sizetype, - adj->offset / BITS_PER_UNIT)); - if (!adj->by_ref) - expr = fold_build1 (INDIRECT_REF, adj->type, expr); + HOST_WIDE_INT base_offset; + tree prev_base; + + if (TREE_CODE (base) == ADDR_EXPR) + base = TREE_OPERAND (base, 0); + prev_base = base; + base = get_addr_base_and_unit_offset (base, &base_offset); + /* Aggregate arguments can have non-invariant addresses. */ + if (!base) + { + base = build_fold_addr_expr (prev_base); + off = build_int_cst (reference_alias_ptr_type (prev_base), + adj->offset / BITS_PER_UNIT); + } + else if (TREE_CODE (base) == MEM_REF) + { + off = build_int_cst (TREE_TYPE (TREE_OPERAND (base,1)), + base_offset + + adj->offset / BITS_PER_UNIT); + off = int_const_binop (PLUS_EXPR, TREE_OPERAND (base, 1), + off, 0); + base = TREE_OPERAND (base, 0); + } + else + { + off = build_int_cst (reference_alias_ptr_type (base), + base_offset + + adj->offset / BITS_PER_UNIT); + base = build_fold_addr_expr (base); + } } + + expr = fold_build2_loc (loc, MEM_REF, adj->type, base, off); + if (adj->by_ref) + expr = build_fold_addr_expr (expr); + expr = force_gimple_operand_gsi (&gsi, expr, adj->by_ref || is_gimple_reg_type (adj->type), Index: mine/gcc/testsuite/g++.dg/torture/pr34850.C =================================================================== --- mine.orig/gcc/testsuite/g++.dg/torture/pr34850.C +++ mine/gcc/testsuite/g++.dg/torture/pr34850.C @@ -11,7 +11,7 @@ extern "C" { extern __inline __attribute__ ((__always_inline__)) __attribute__ ((__gnu_inline__, __artificial__)) void * memset (void *__dest, int __ch, size_t __len) throw () { if (__builtin_constant_p (__len) && __len == 0) - __warn_memset_zero_len (); /* { dg-warning "" } */ + __warn_memset_zero_len (); } } inline void clear_mem(void* ptr, u32bit n) { ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Remove the IPA-SRA call to build_ref_for_offset 2010-09-08 17:16 ` [PATCH 1/2] Remove the IPA-SRA call to build_ref_for_offset Martin Jambor @ 2010-09-09 9:01 ` Richard Guenther 2010-09-11 2:09 ` H.J. Lu 1 sibling, 0 replies; 11+ messages in thread From: Richard Guenther @ 2010-09-09 9:01 UTC (permalink / raw) To: Martin Jambor; +Cc: GCC Patches On Wed, 8 Sep 2010, Martin Jambor wrote: > Hi, > > this is the same patch I sent here in August: > http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00609.html > > It reimplements parameter alterations in callers of a function being > modified by IPA-SRA so that it always produces MEM_REF relying on > force_gimple_operand_gsi to break up refs if necessary (e.g. if we > have a mem_ref of an addr_expr of an array_ref with variable index of > another mem_ref etc...). > > I have bootstrapped and tested this patch separately on x86_64-linux. > OK for trunk? Ok. Thanks, Richard. > Thanks, > > Martin > > > 2010-08-08 Martin Jambor <mjambor@suse.cz> > > PR tree-optimization/44972 > * ipa-prop.c (ipa_modify_call_arguments): Build MEM_REF instead of > calling build_ref_for_offset. > > * testsuite/g++.dg/torture/pr34850.C: Remove expected warning. > > Index: mine/gcc/ipa-prop.c > =================================================================== > --- mine.orig/gcc/ipa-prop.c > +++ mine/gcc/ipa-prop.c > @@ -2153,40 +2153,77 @@ ipa_modify_call_arguments (struct cgraph > } > else if (!adj->remove_param) > { > - tree expr, orig_expr; > - bool allow_ptr, repl_found; > + tree expr, base, off; > + location_t loc; > > - orig_expr = expr = gimple_call_arg (stmt, adj->base_index); > - if (TREE_CODE (expr) == ADDR_EXPR) > - { > - allow_ptr = false; > - expr = TREE_OPERAND (expr, 0); > - } > - else > - allow_ptr = true; > + /* We create a new parameter out of the value of the old one, we can > + do the following kind of transformations: > > - repl_found = build_ref_for_offset (&expr, TREE_TYPE (expr), > - adj->offset, adj->type, > - allow_ptr); > - if (repl_found) > - { > - if (adj->by_ref) > - expr = build_fold_addr_expr (expr); > - } > + - A scalar passed by reference is converted to a scalar passed by > + value. (adj->by_ref is false and the type of the original > + actual argument is a pointer to a scalar). > + > + - A part of an aggregate is passed instead of the whole aggregate. > + The part can be passed either by value or by reference, this is > + determined by value of adj->by_ref. Moreover, the code below > + handles both situations when the original aggregate is passed by > + value (its type is not a pointer) and when it is passed by > + reference (it is a pointer to an aggregate). > + > + When the new argument is passed by reference (adj->by_ref is true) > + it must be a part of an aggregate and therefore we form it by > + simply taking the address of a reference inside the original > + aggregate. */ > + > + gcc_checking_assert (adj->offset % BITS_PER_UNIT == 0); > + base = gimple_call_arg (stmt, adj->base_index); > + loc = EXPR_LOCATION (base); > + > + if (TREE_CODE (base) == ADDR_EXPR > + && DECL_P (TREE_OPERAND (base, 0))) > + off = build_int_cst (reference_alias_ptr_type (base), > + adj->offset / BITS_PER_UNIT); > + else if (TREE_CODE (base) != ADDR_EXPR > + && POINTER_TYPE_P (TREE_TYPE (base))) > + off = build_int_cst (TREE_TYPE (base), adj->offset / BITS_PER_UNIT); > else > { > - tree ptrtype = build_pointer_type (adj->type); > - expr = orig_expr; > - if (!POINTER_TYPE_P (TREE_TYPE (expr))) > - expr = build_fold_addr_expr (expr); > - if (!useless_type_conversion_p (ptrtype, TREE_TYPE (expr))) > - expr = fold_convert (ptrtype, expr); > - expr = fold_build2 (POINTER_PLUS_EXPR, ptrtype, expr, > - build_int_cst (sizetype, > - adj->offset / BITS_PER_UNIT)); > - if (!adj->by_ref) > - expr = fold_build1 (INDIRECT_REF, adj->type, expr); > + HOST_WIDE_INT base_offset; > + tree prev_base; > + > + if (TREE_CODE (base) == ADDR_EXPR) > + base = TREE_OPERAND (base, 0); > + prev_base = base; > + base = get_addr_base_and_unit_offset (base, &base_offset); > + /* Aggregate arguments can have non-invariant addresses. */ > + if (!base) > + { > + base = build_fold_addr_expr (prev_base); > + off = build_int_cst (reference_alias_ptr_type (prev_base), > + adj->offset / BITS_PER_UNIT); > + } > + else if (TREE_CODE (base) == MEM_REF) > + { > + off = build_int_cst (TREE_TYPE (TREE_OPERAND (base,1)), > + base_offset > + + adj->offset / BITS_PER_UNIT); > + off = int_const_binop (PLUS_EXPR, TREE_OPERAND (base, 1), > + off, 0); > + base = TREE_OPERAND (base, 0); > + } > + else > + { > + off = build_int_cst (reference_alias_ptr_type (base), > + base_offset > + + adj->offset / BITS_PER_UNIT); > + base = build_fold_addr_expr (base); > + } > } > + > + expr = fold_build2_loc (loc, MEM_REF, adj->type, base, off); > + if (adj->by_ref) > + expr = build_fold_addr_expr (expr); > + > expr = force_gimple_operand_gsi (&gsi, expr, > adj->by_ref > || is_gimple_reg_type (adj->type), > Index: mine/gcc/testsuite/g++.dg/torture/pr34850.C > =================================================================== > --- mine.orig/gcc/testsuite/g++.dg/torture/pr34850.C > +++ mine/gcc/testsuite/g++.dg/torture/pr34850.C > @@ -11,7 +11,7 @@ extern "C" { > extern __inline __attribute__ ((__always_inline__)) __attribute__ ((__gnu_inline__, __artificial__)) > void * memset (void *__dest, int __ch, size_t __len) throw () { > if (__builtin_constant_p (__len) && __len == 0) > - __warn_memset_zero_len (); /* { dg-warning "" } */ > + __warn_memset_zero_len (); > } > } > inline void clear_mem(void* ptr, u32bit n) { > > -- Richard Guenther <rguenther@suse.de> Novell / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Remove the IPA-SRA call to build_ref_for_offset 2010-09-08 17:16 ` [PATCH 1/2] Remove the IPA-SRA call to build_ref_for_offset Martin Jambor 2010-09-09 9:01 ` Richard Guenther @ 2010-09-11 2:09 ` H.J. Lu 2010-12-06 18:56 ` H.J. Lu 1 sibling, 1 reply; 11+ messages in thread From: H.J. Lu @ 2010-09-11 2:09 UTC (permalink / raw) To: Martin Jambor; +Cc: GCC Patches, Richard Guenther On Wed, Sep 8, 2010 at 9:43 AM, Martin Jambor <mjambor@suse.cz> wrote: > Hi, > > this is the same patch I sent here in August: > http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00609.html > > It reimplements parameter alterations in callers of a function being > modified by IPA-SRA so that it always produces MEM_REF relying on > force_gimple_operand_gsi to break up refs if necessary (e.g. if we > have a mem_ref of an addr_expr of an array_ref with variable index of > another mem_ref etc...). > > I have bootstrapped and tested this patch separately on x86_64-linux. > OK for trunk? > > Thanks, > > Martin > > > 2010-08-08 Martin Jambor <mjambor@suse.cz> > > PR tree-optimization/44972 > * ipa-prop.c (ipa_modify_call_arguments): Build MEM_REF instead of > calling build_ref_for_offset. > > * testsuite/g++.dg/torture/pr34850.C: Remove expected warning. > This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45644 -- H.J. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Remove the IPA-SRA call to build_ref_for_offset 2010-09-11 2:09 ` H.J. Lu @ 2010-12-06 18:56 ` H.J. Lu 2011-01-13 23:59 ` H.J. Lu 0 siblings, 1 reply; 11+ messages in thread From: H.J. Lu @ 2010-12-06 18:56 UTC (permalink / raw) To: Martin Jambor; +Cc: GCC Patches, Richard Guenther On Fri, Sep 10, 2010 at 5:21 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Sep 8, 2010 at 9:43 AM, Martin Jambor <mjambor@suse.cz> wrote: >> Hi, >> >> this is the same patch I sent here in August: >> http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00609.html >> >> It reimplements parameter alterations in callers of a function being >> modified by IPA-SRA so that it always produces MEM_REF relying on >> force_gimple_operand_gsi to break up refs if necessary (e.g. if we >> have a mem_ref of an addr_expr of an array_ref with variable index of >> another mem_ref etc...). >> >> I have bootstrapped and tested this patch separately on x86_64-linux. >> OK for trunk? >> >> Thanks, >> >> Martin >> >> >> 2010-08-08 Martin Jambor <mjambor@suse.cz> >> >> PR tree-optimization/44972 >> * ipa-prop.c (ipa_modify_call_arguments): Build MEM_REF instead of >> calling build_ref_for_offset. >> >> * testsuite/g++.dg/torture/pr34850.C: Remove expected warning. >> > > This caused: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45644 > This also caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46823 -- H.J. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Remove the IPA-SRA call to build_ref_for_offset 2010-12-06 18:56 ` H.J. Lu @ 2011-01-13 23:59 ` H.J. Lu 0 siblings, 0 replies; 11+ messages in thread From: H.J. Lu @ 2011-01-13 23:59 UTC (permalink / raw) To: Martin Jambor; +Cc: GCC Patches, Richard Guenther On Mon, Dec 6, 2010 at 10:56 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Fri, Sep 10, 2010 at 5:21 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Wed, Sep 8, 2010 at 9:43 AM, Martin Jambor <mjambor@suse.cz> wrote: >>> Hi, >>> >>> this is the same patch I sent here in August: >>> http://gcc.gnu.org/ml/gcc-patches/2010-08/msg00609.html >>> >>> It reimplements parameter alterations in callers of a function being >>> modified by IPA-SRA so that it always produces MEM_REF relying on >>> force_gimple_operand_gsi to break up refs if necessary (e.g. if we >>> have a mem_ref of an addr_expr of an array_ref with variable index of >>> another mem_ref etc...). >>> >>> I have bootstrapped and tested this patch separately on x86_64-linux. >>> OK for trunk? >>> >>> Thanks, >>> >>> Martin >>> >>> >>> 2010-08-08 Martin Jambor <mjambor@suse.cz> >>> >>> PR tree-optimization/44972 >>> * ipa-prop.c (ipa_modify_call_arguments): Build MEM_REF instead of >>> calling build_ref_for_offset. >>> >>> * testsuite/g++.dg/torture/pr34850.C: Remove expected warning. >>> >> >> This caused: >> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45644 >> > > This also caused: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46823 > This also caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47283 -- H.J. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] Reimplementation of build_ref_for_offset 2010-09-08 17:16 [PATCH 0/2] Fix PR 44972 Martin Jambor 2010-09-08 17:16 ` [PATCH 1/2] Remove the IPA-SRA call to build_ref_for_offset Martin Jambor @ 2010-09-08 17:19 ` Martin Jambor 2010-09-09 9:31 ` Richard Guenther 2010-10-23 19:33 ` H.J. Lu 1 sibling, 2 replies; 11+ messages in thread From: Martin Jambor @ 2010-09-08 17:19 UTC (permalink / raw) To: GCC Patches; +Cc: Richard Guenther [-- Attachment #1: rework-build_ref_for_offset.diff --] [-- Type: text/plain, Size: 29383 bytes --] Hi, this patch reimplements build_ref_for_offset so that it simply creates a MEM_REF rather than trying to figure out what combination of component and array refs are necessary. The main advantage of this approach is that this can never fail, allowing us to be more aggressive and remove a number of checks. There were two main problems with this, though. First is that MEM_REFs are not particularly readable to by users. This would be a problem when we are creating a reference that might be displayed to them in a warning or a debugger which is what we do with DECL_DEBUG_EXPR expressions. We sometimes construct these artificially when propagating accesses across assignments. So for those cases I retained the old implementation and only simplified it a bit - it is now called build_user_friendly_ref_for_offset. The other problem was bit-fields. Constructing accesses to them was difficult enough but then I realized that I was not even able to detect the cases when I was accessing a bit field if their offset happened to be on a byte boundary. I thought I would be able to figure this out from TYPE_SIZE and TYPE_PRECISION of exp_type but combinations that signal a bit-field in one language may not be applied in another (in C, small TYPE_PRECISION denotes bit-fields and TYPE_SIZE is big, but for example Fortran booleans have the precision set to one even though they are not bit-fields). So in the end I based the detection on the access structures that represented the thing being loaded or stored which I knew had their sizes correct because they are based on field sizes. Since I use the access, the simplest way to actually create the reference to the bit field is to re-use the last component ref of its expression - that is what build_ref_for_model (meaning a model access) does. Separating this from build_ref_for_offset (which cannot handle bit-fields) makes the code a bit cleaner and keeps the latter function for other users which know nothing about SRA access structures. I hope that you'll find these approaches reasonable. The patch was bootstrapped and tested on x86_64-linux without any issues. I'd like to commit it to trunk but I'm sure there will be comments and suggestions. Thanks, Martin 2010-09-08 Martin Jambor <mjambor@suse.cz> PR tree-optimization/44972 * tree-sra.c: Include toplev.h. (build_ref_for_offset): Entirely reimplemented. (build_ref_for_model): New function. (build_user_friendly_ref_for_offset): New function. (analyze_access_subtree): Removed build_ref_for_offset check. (propagate_subaccesses_across_link): Likewise. (create_artificial_child_access): Use build_user_friendly_ref_for_offset. (propagate_subaccesses_across_link): Likewise. (ref_expr_for_all_replacements_p): Removed. (generate_subtree_copies): Updated comment. Use build_ref_for_model. (sra_modify_expr): Use build_ref_for_model. (load_assign_lhs_subreplacements): Likewise. (sra_modify_assign): Removed ref_expr_for_all_replacements_p checks, checks for return values of build_ref_for_offset. * ipa-cp.c (ipcp_lattice_from_jfunc): No need to check return value of build_ref_for_offset. * ipa-prop.h: Include gimple.h * ipa-prop.c (ipa_compute_jump_functions): Update to look for MEM_REFs. (ipa_analyze_indirect_call_uses): Update comment. * Makefile.in (tree-sra.o): Add $(GIMPLE_H) to dependencies. (IPA_PROP_H): Likewise. * testsuite/gcc.dg/ipa/ipa-sra-1.c: Adjust scanning expressions. * testsuite/gcc.dg/tree-ssa/pr45144.c: Likewise. * testsuite/gcc.dg/tree-ssa/forwprop-5.c: Likewise and scan optimzed dump instead. * testsuite/g++.dg/torture/pr34850.C: Remove expected warning. * gcc/testsuite/g++.dg/torture/pr44972.C: New test. Index: mine/gcc/tree-sra.c =================================================================== --- mine.orig/gcc/tree-sra.c +++ mine/gcc/tree-sra.c @@ -76,6 +76,7 @@ along with GCC; see the file COPYING3. #include "coretypes.h" #include "alloc-pool.h" #include "tm.h" +#include "toplev.h" #include "tree.h" #include "gimple.h" #include "cgraph.h" @@ -1320,15 +1321,114 @@ make_fancy_name (tree expr) return XOBFINISH (&name_obstack, char *); } -/* Helper function for build_ref_for_offset. +/* Construct a MEM_REF that would reference a part of aggregate BASE of type + EXP_TYPE at the given OFFSET. If BASE is something for which + get_addr_base_and_unit_offset returns NULL, gsi must be non-NULL and is used + to insert new statements either before or below the current one as specified + by INSERT_AFTER. If offset is not aligned to bytes or EXP_TYPE is a + bit_field. This function is not capable of handling bitfields. */ + +tree +build_ref_for_offset (tree base, HOST_WIDE_INT offset, + tree exp_type, gimple_stmt_iterator *gsi, + bool insert_after) +{ + tree prev_base = base; + tree off; + location_t loc = EXPR_LOCATION (base); + HOST_WIDE_INT base_offset; - FIXME: Eventually this should be rewritten to either re-use the - original access expression unshared (which is good for alias - analysis) or to build a MEM_REF expression. */ + gcc_checking_assert (offset % BITS_PER_UNIT == 0); + + base = get_addr_base_and_unit_offset (base, &base_offset); + if (!base) + { + gimple stmt; + tree tmp, addr; + + gcc_checking_assert (gsi); + tmp = create_tmp_reg (build_pointer_type (TREE_TYPE (prev_base)), NULL); + add_referenced_var (tmp); + tmp = make_ssa_name (tmp, NULL); + addr = build_fold_addr_expr (unshare_expr (prev_base)); + stmt = gimple_build_assign (tmp, addr); + SSA_NAME_DEF_STMT (tmp) = stmt; + if (insert_after) + gsi_insert_after (gsi, stmt, GSI_NEW_STMT); + else + gsi_insert_before (gsi, stmt, GSI_SAME_STMT); + + off = build_int_cst (reference_alias_ptr_type (prev_base), + offset / BITS_PER_UNIT); + base = tmp; + } + else if (TREE_CODE (base) == MEM_REF) + { + off = build_int_cst (TREE_TYPE (TREE_OPERAND (base, 1)), + base_offset + offset / BITS_PER_UNIT); + off = int_const_binop (PLUS_EXPR, TREE_OPERAND (base, 1), off, 0); + base = unshare_expr (TREE_OPERAND (base, 0)); + } + else + { + off = build_int_cst (reference_alias_ptr_type (base), + base_offset + offset / BITS_PER_UNIT); + base = build_fold_addr_expr (unshare_expr (base)); + } + + return fold_build2_loc (loc, MEM_REF, exp_type, base, off); +} + +/* Construct a memory reference to a part of an aggregate BASE at the given + OFFSET and of the same type as MODEL. In case this is a reference to a + bit-field, the function will replicate the last component_ref of model's + expr to access it. GSI and INSERT_AFTER have the same meaning as in + build_ref_for_offset. */ + +static tree +build_ref_for_model (tree base, HOST_WIDE_INT offset, + struct access *model, gimple_stmt_iterator *gsi, + bool insert_after) +{ + tree t, exp_type; + bool bitfield; + + if (offset % BITS_PER_UNIT != 0 + || model->size < BITS_PER_UNIT + || exact_log2 (model->size) == -1) + { + gcc_checking_assert (TREE_CODE (model->expr) == COMPONENT_REF); + offset -= int_bit_position (TREE_OPERAND (model->expr, 1)); + gcc_assert (offset % BITS_PER_UNIT == 0); + exp_type = TREE_TYPE (TREE_OPERAND (model->expr, 0)); + bitfield = true; + } + else + { + exp_type = model->type; + bitfield = false; + } + + t = build_ref_for_offset (base, offset, exp_type, gsi, insert_after); + + if (bitfield) + t = fold_build3_loc (EXPR_LOCATION (base), COMPONENT_REF, model->type, t, + TREE_OPERAND (model->expr, 1), NULL_TREE); + + return t; +} + +/* Construct a memory reference consisting of component_refs and array_refs to + a part of an aggregate *RES (which is of type TYPE). The requested part + should have type EXP_TYPE at be the given OFFSET. This function might not + succeed, it returns true when it does and only then *RES points to something + meaningful. This function should be used only to build expressions that we + might need to present to user (e.g. in warnings). In all other situations, + build_ref_for_model or build_ref_for_offset should be used instead. */ static bool -build_ref_for_offset_1 (tree *res, tree type, HOST_WIDE_INT offset, - tree exp_type) +build_user_friendly_ref_for_offset (tree *res, tree type, HOST_WIDE_INT offset, + tree exp_type) { while (1) { @@ -1367,19 +1467,13 @@ build_ref_for_offset_1 (tree *res, tree else if (pos > offset || (pos + size) <= offset) continue; - if (res) + expr = build3 (COMPONENT_REF, TREE_TYPE (fld), *res, fld, + NULL_TREE); + expr_ptr = &expr; + if (build_user_friendly_ref_for_offset (expr_ptr, TREE_TYPE (fld), + offset - pos, exp_type)) { - expr = build3 (COMPONENT_REF, TREE_TYPE (fld), *res, fld, - NULL_TREE); - expr_ptr = &expr; - } - else - expr_ptr = NULL; - if (build_ref_for_offset_1 (expr_ptr, TREE_TYPE (fld), - offset - pos, exp_type)) - { - if (res) - *res = expr; + *res = expr; return true; } } @@ -1394,14 +1488,11 @@ build_ref_for_offset_1 (tree *res, tree minidx = TYPE_MIN_VALUE (TYPE_DOMAIN (type)); if (TREE_CODE (minidx) != INTEGER_CST || el_size == 0) return false; - if (res) - { - index = build_int_cst (TYPE_DOMAIN (type), offset / el_size); - if (!integer_zerop (minidx)) - index = int_const_binop (PLUS_EXPR, index, minidx, 0); - *res = build4 (ARRAY_REF, TREE_TYPE (type), *res, index, - NULL_TREE, NULL_TREE); - } + index = build_int_cst (TYPE_DOMAIN (type), offset / el_size); + if (!integer_zerop (minidx)) + index = int_const_binop (PLUS_EXPR, index, minidx, 0); + *res = build4 (ARRAY_REF, TREE_TYPE (type), *res, index, + NULL_TREE, NULL_TREE); offset = offset % el_size; type = TREE_TYPE (type); break; @@ -1418,31 +1509,6 @@ build_ref_for_offset_1 (tree *res, tree } } -/* Construct an expression that would reference a part of aggregate *EXPR of - type TYPE at the given OFFSET of the type EXP_TYPE. If EXPR is NULL, the - function only determines whether it can build such a reference without - actually doing it, otherwise, the tree it points to is unshared first and - then used as a base for furhter sub-references. */ - -bool -build_ref_for_offset (tree *expr, tree type, HOST_WIDE_INT offset, - tree exp_type, bool allow_ptr) -{ - location_t loc = expr ? EXPR_LOCATION (*expr) : UNKNOWN_LOCATION; - - if (expr) - *expr = unshare_expr (*expr); - - if (allow_ptr && POINTER_TYPE_P (type)) - { - type = TREE_TYPE (type); - if (expr) - *expr = build_simple_mem_ref_loc (loc, *expr); - } - - return build_ref_for_offset_1 (expr, type, offset, exp_type); -} - /* Return true iff TYPE is stdarg va_list type. */ static inline bool @@ -1823,13 +1889,7 @@ analyze_access_subtree (struct access *r if (allow_replacements && scalar && !root->first_child && (root->grp_hint - || (root->grp_write && (direct_read || root->grp_assignment_read))) - /* We must not ICE later on when trying to build an access to the - original data within the aggregate even when it is impossible to do in - a defined way like in the PR 42703 testcase. Therefore we check - pre-emptively here that we will be able to do that. */ - && build_ref_for_offset (NULL, TREE_TYPE (root->base), root->offset, - root->type, false)) + || (root->grp_write && (direct_read || root->grp_assignment_read)))) { if (dump_file && (dump_flags & TDF_DETAILS)) { @@ -1914,12 +1974,11 @@ create_artificial_child_access (struct a { struct access *access; struct access **child; - tree expr = parent->base;; + tree expr = parent->base; gcc_assert (!model->grp_unscalarizable_region); - - if (!build_ref_for_offset (&expr, TREE_TYPE (expr), new_offset, - model->type, false)) + if (!build_user_friendly_ref_for_offset (&expr, TREE_TYPE (expr), new_offset, + model->type)) return NULL; access = (struct access *) pool_alloc (access_pool); @@ -1964,8 +2023,8 @@ propagate_subaccesses_across_link (struc { tree t = lacc->base; - if (build_ref_for_offset (&t, TREE_TYPE (t), lacc->offset, racc->type, - false)) + if (build_user_friendly_ref_for_offset (&t, TREE_TYPE (t), lacc->offset, + racc->type)) { lacc->expr = t; lacc->type = racc->type; @@ -1994,13 +2053,6 @@ propagate_subaccesses_across_link (struc continue; } - /* If a (part of) a union field is on the RHS of an assignment, it can - have sub-accesses which do not make sense on the LHS (PR 40351). - Check that this is not the case. */ - if (!build_ref_for_offset (NULL, TREE_TYPE (lacc->base), norm_offset, - rchild->type, false)) - continue; - rchild->grp_hint = 1; new_acc = create_artificial_child_access (lacc, rchild, norm_offset); if (new_acc) @@ -2124,48 +2176,19 @@ analyze_all_variable_accesses (void) return false; } -/* Return true iff a reference statement into aggregate AGG can be built for - every single to-be-replaced accesses that is a child of ACCESS, its sibling - or a child of its sibling. TOP_OFFSET is the offset from the processed - access subtree that has to be subtracted from offset of each access. */ - -static bool -ref_expr_for_all_replacements_p (struct access *access, tree agg, - HOST_WIDE_INT top_offset) -{ - do - { - if (access->grp_to_be_replaced - && !build_ref_for_offset (NULL, TREE_TYPE (agg), - access->offset - top_offset, - access->type, false)) - return false; - - if (access->first_child - && !ref_expr_for_all_replacements_p (access->first_child, agg, - top_offset)) - return false; - - access = access->next_sibling; - } - while (access); - - return true; -} - /* Generate statements copying scalar replacements of accesses within a subtree into or out of AGG. ACCESS is the first child of the root of the subtree to be processed. AGG is an aggregate type expression (can be a declaration but - does not have to be, it can for example also be an indirect_ref). - TOP_OFFSET is the offset of the processed subtree which has to be subtracted - from offsets of individual accesses to get corresponding offsets for AGG. - If CHUNK_SIZE is non-null, copy only replacements in the interval - <start_offset, start_offset + chunk_size>, otherwise copy all. GSI is a - statement iterator used to place the new statements. WRITE should be true - when the statements should write from AGG to the replacement and false if - vice versa. if INSERT_AFTER is true, new statements will be added after the - current statement in GSI, they will be added before the statement - otherwise. */ + does not have to be, it can for example also be a mem_ref or a series of + handled components). TOP_OFFSET is the offset of the processed subtree + which has to be subtracted from offsets of individual accesses to get + corresponding offsets for AGG. If CHUNK_SIZE is non-null, copy only + replacements in the interval <start_offset, start_offset + chunk_size>, + otherwise copy all. GSI is a statement iterator used to place the new + statements. WRITE should be true when the statements should write from AGG + to the replacement and false if vice versa. if INSERT_AFTER is true, new + statements will be added after the current statement in GSI, they will be + added before the statement otherwise. */ static void generate_subtree_copies (struct access *access, tree agg, @@ -2176,8 +2199,6 @@ generate_subtree_copies (struct access * { do { - tree expr = agg; - if (chunk_size && access->offset >= start_offset + chunk_size) return; @@ -2185,14 +2206,11 @@ generate_subtree_copies (struct access * && (chunk_size == 0 || access->offset + access->size > start_offset)) { - tree repl = get_access_replacement (access); - bool ref_found; + tree expr, repl = get_access_replacement (access); gimple stmt; - ref_found = build_ref_for_offset (&expr, TREE_TYPE (agg), - access->offset - top_offset, - access->type, false); - gcc_assert (ref_found); + expr = build_ref_for_model (agg, access->offset - top_offset, + access, gsi, insert_after); if (write) { @@ -2329,12 +2347,10 @@ sra_modify_expr (tree *expr, gimple_stmt in assembler statements (see PR42398). */ if (!useless_type_conversion_p (type, access->type)) { - tree ref = access->base; - bool ok; + tree ref; - ok = build_ref_for_offset (&ref, TREE_TYPE (ref), - access->offset, access->type, false); - gcc_assert (ok); + ref = build_ref_for_model (access->base, access->offset, access, + NULL, false); if (write) { @@ -2458,25 +2474,11 @@ load_assign_lhs_subreplacements (struct lhs, old_gsi); if (*refreshed == SRA_UDH_LEFT) - { - bool repl_found; - - rhs = lacc->base; - repl_found = build_ref_for_offset (&rhs, TREE_TYPE (rhs), - lacc->offset, lacc->type, - false); - gcc_assert (repl_found); - } + rhs = build_ref_for_model (lacc->base, lacc->offset, lacc, + new_gsi, true); else - { - bool repl_found; - - rhs = top_racc->base; - repl_found = build_ref_for_offset (&rhs, - TREE_TYPE (top_racc->base), - offset, lacc->type, false); - gcc_assert (repl_found); - } + rhs = build_ref_for_model (top_racc->base, offset, lacc, + new_gsi, true); } stmt = gimple_build_assign (get_access_replacement (lacc), rhs); @@ -2633,25 +2635,18 @@ sra_modify_assign (gimple *stmt, gimple_ if (AGGREGATE_TYPE_P (TREE_TYPE (lhs)) && !access_has_children_p (lacc)) { - tree expr = lhs; - if (build_ref_for_offset (&expr, TREE_TYPE (lhs), 0, - TREE_TYPE (rhs), false)) - { - lhs = expr; - gimple_assign_set_lhs (*stmt, expr); - } + lhs = build_ref_for_offset (lhs, 0, TREE_TYPE (rhs), gsi, false); + gimple_assign_set_lhs (*stmt, lhs); } else if (AGGREGATE_TYPE_P (TREE_TYPE (rhs)) + && !contains_view_convert_expr_p (rhs) && !access_has_children_p (racc)) - { - tree expr = rhs; - if (build_ref_for_offset (&expr, TREE_TYPE (rhs), 0, - TREE_TYPE (lhs), false)) - rhs = expr; - } + rhs = build_ref_for_offset (rhs, 0, TREE_TYPE (lhs), gsi, false); + if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs))) { - rhs = fold_build1_loc (loc, VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs); + rhs = fold_build1_loc (loc, VIEW_CONVERT_EXPR, TREE_TYPE (lhs), + rhs); if (is_gimple_reg_type (TREE_TYPE (lhs)) && TREE_CODE (lhs) != SSA_NAME) force_gimple_rhs = true; @@ -2694,11 +2689,7 @@ sra_modify_assign (gimple *stmt, gimple_ if (gimple_has_volatile_ops (*stmt) || contains_view_convert_expr_p (rhs) - || contains_view_convert_expr_p (lhs) - || (access_has_children_p (racc) - && !ref_expr_for_all_replacements_p (racc, lhs, racc->offset)) - || (access_has_children_p (lacc) - && !ref_expr_for_all_replacements_p (lacc, rhs, lacc->offset))) + || contains_view_convert_expr_p (lhs)) { if (access_has_children_p (racc)) generate_subtree_copies (racc->first_child, racc->base, 0, 0, 0, Index: mine/gcc/ipa-cp.c =================================================================== --- mine.orig/gcc/ipa-cp.c +++ mine/gcc/ipa-cp.c @@ -327,7 +327,6 @@ ipcp_lattice_from_jfunc (struct ipa_node { struct ipcp_lattice *caller_lat; tree t; - bool ok; caller_lat = ipcp_get_lattice (info, jfunc->value.ancestor.formal_id); lat->type = caller_lat->type; @@ -340,16 +339,9 @@ ipcp_lattice_from_jfunc (struct ipa_node return; } t = TREE_OPERAND (caller_lat->constant, 0); - ok = build_ref_for_offset (&t, TREE_TYPE (t), - jfunc->value.ancestor.offset, - jfunc->value.ancestor.type, false); - if (!ok) - { - lat->type = IPA_BOTTOM; - lat->constant = NULL_TREE; - } - else - lat->constant = build_fold_addr_expr (t); + t = build_ref_for_offset (t, jfunc->value.ancestor.offset, + jfunc->value.ancestor.type, NULL, false); + lat->constant = build_fold_addr_expr (t); } else lat->type = IPA_BOTTOM; Index: mine/gcc/ipa-prop.h =================================================================== --- mine.orig/gcc/ipa-prop.h +++ mine/gcc/ipa-prop.h @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3. #include "tree.h" #include "vec.h" #include "cgraph.h" +#include "gimple.h" /* The following definitions and interfaces are used by interprocedural analyses or parameters. */ @@ -511,6 +512,7 @@ void ipa_prop_read_jump_functions (void) void ipa_update_after_lto_read (void); /* From tree-sra.c: */ -bool build_ref_for_offset (tree *, tree, HOST_WIDE_INT, tree, bool); +tree build_ref_for_offset (tree, HOST_WIDE_INT, tree, gimple_stmt_iterator *, + bool); #endif /* IPA_PROP_H */ Index: mine/gcc/Makefile.in =================================================================== --- mine.orig/gcc/Makefile.in +++ mine/gcc/Makefile.in @@ -968,7 +968,7 @@ EBITMAP_H = ebitmap.h sbitmap.h LTO_STREAMER_H = lto-streamer.h $(LINKER_PLUGIN_API_H) $(TARGET_H) \ $(CGRAPH_H) $(VEC_H) vecprim.h $(TREE_H) $(GIMPLE_H) TREE_VECTORIZER_H = tree-vectorizer.h $(TREE_DATA_REF_H) -IPA_PROP_H = ipa-prop.h $(TREE_H) $(VEC_H) $(CGRAPH_H) +IPA_PROP_H = ipa-prop.h $(TREE_H) $(VEC_H) $(CGRAPH_H) $(GIMPLE_H) GSTAB_H = gstab.h stab.def BITMAP_H = bitmap.h $(HASHTAB_H) statistics.h GCC_PLUGIN_H = gcc-plugin.h highlev-plugin-common.h $(CONFIG_H) $(SYSTEM_H) \ @@ -3142,10 +3142,10 @@ tree-ssa-ccp.o : tree-ssa-ccp.c $(TREE_F tree-ssa-propagate.h value-prof.h $(FLAGS_H) $(TARGET_H) $(TOPLEV_H) $(DIAGNOSTIC_CORE_H) \ $(DBGCNT_H) tree-pretty-print.h gimple-pretty-print.h tree-sra.o : tree-sra.c $(CONFIG_H) $(SYSTEM_H) coretypes.h alloc-pool.h \ - $(TM_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) $(IPA_PROP_H) \ - $(DIAGNOSTIC_H) statistics.h $(TREE_DUMP_H) $(TIMEVAR_H) $(PARAMS_H) \ - $(TARGET_H) $(FLAGS_H) $(EXPR_H) tree-pretty-print.h $(DBGCNT_H) \ - $(TREE_INLINE_H) gimple-pretty-print.h + $(TM_H) $(TOPLEV_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) \ + $(IPA_PROP_H) $(DIAGNOSTIC_H) statistics.h $(TREE_DUMP_H) $(TIMEVAR_H) \ + $(PARAMS_H) $(TARGET_H) $(FLAGS_H) $(EXPR_H) tree-pretty-print.h \ + $(DBGCNT_H) $(TREE_INLINE_H) gimple-pretty-print.h tree-switch-conversion.o : tree-switch-conversion.c $(CONFIG_H) $(SYSTEM_H) \ $(TREE_H) $(TM_P_H) $(TREE_FLOW_H) $(DIAGNOSTIC_H) $(TREE_INLINE_H) \ $(TIMEVAR_H) $(TM_H) coretypes.h $(TREE_DUMP_H) $(GIMPLE_H) \ Index: mine/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c =================================================================== --- mine.orig/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c +++ mine/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c @@ -36,6 +36,5 @@ main (int argc, char *argv[]) return 0; } -/* { dg-final { scan-tree-dump "About to replace expr cow.green with ISRA" "eipa_sra" } } */ -/* { dg-final { scan-tree-dump "About to replace expr cow.blue with ISRA" "eipa_sra" } } */ +/* { dg-final { scan-tree-dump-times "About to replace expr" 2 "eipa_sra" } } */ /* { dg-final { cleanup-tree-dump "eipa_sra" } } */ Index: mine/gcc/testsuite/gcc.dg/tree-ssa/forwprop-5.c =================================================================== --- mine.orig/gcc/testsuite/gcc.dg/tree-ssa/forwprop-5.c +++ mine/gcc/testsuite/gcc.dg/tree-ssa/forwprop-5.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O1 -fdump-tree-esra -w" } */ +/* { dg-options "-O1 -fdump-tree-optimized -w" } */ #define vector __attribute__((vector_size(16) )) struct VecClass @@ -11,12 +11,9 @@ vector float foo( vector float v ) { vector float x = v; x = x + x; - struct VecClass y = *(struct VecClass*)&x; - return y.v; + struct VecClass disappear = *(struct VecClass*)&x; + return disappear.v; } -/* We should be able to remove the intermediate struct and directly - return x. As we do not fold VIEW_CONVERT_EXPR<struct VecClass>(x).v - that doesn't happen right now. */ -/* { dg-final { scan-tree-dump-times "VIEW_CONVERT_EXPR" 1 "esra"} } */ -/* { dg-final { cleanup-tree-dump "esra" } } */ +/* { dg-final { scan-tree-dump-times "disappear" 0 "optimized"} } */ +/* { dg-final { cleanup-tree-dump "optimized" } } */ Index: mine/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c =================================================================== --- mine.orig/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c +++ mine/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c @@ -42,5 +42,5 @@ bar (unsigned orig, unsigned *new) *new = foo (&a); } -/* { dg-final { scan-tree-dump "x = a;" "optimized"} } */ +/* { dg-final { scan-tree-dump " = VIEW_CONVERT_EXPR<unsigned int>\\(a\\);" "optimized"} } */ /* { dg-final { cleanup-tree-dump "optimized" } } */ Index: mine/gcc/ipa-prop.c =================================================================== --- mine.orig/gcc/ipa-prop.c +++ mine/gcc/ipa-prop.c @@ -916,23 +916,27 @@ ipa_compute_jump_functions (struct cgrap static tree ipa_get_member_ptr_load_param (tree rhs, bool use_delta) { - tree rec, fld; + tree rec, ref_offset, fld_offset; tree ptr_field; tree delta_field; - if (TREE_CODE (rhs) != COMPONENT_REF) + if (TREE_CODE (rhs) != MEM_REF) return NULL_TREE; - rec = TREE_OPERAND (rhs, 0); + if (TREE_CODE (rec) != ADDR_EXPR) + return NULL_TREE; + rec = TREE_OPERAND (rec, 0); if (TREE_CODE (rec) != PARM_DECL || !type_like_member_ptr_p (TREE_TYPE (rec), &ptr_field, &delta_field)) return NULL_TREE; - fld = TREE_OPERAND (rhs, 1); - if (use_delta ? (fld == delta_field) : (fld == ptr_field)) - return rec; + ref_offset = TREE_OPERAND (rhs, 1); + if (use_delta) + fld_offset = byte_position (delta_field); else - return NULL_TREE; + fld_offset = byte_position (ptr_field); + + return tree_int_cst_equal (ref_offset, fld_offset) ? rec : NULL_TREE; } /* If STMT looks like a statement loading a value from a member pointer formal @@ -999,8 +1003,8 @@ ipa_note_param_call (struct cgraph_node below, the call is on the last line: <bb 2>: - f$__delta_5 = f.__delta; - f$__pfn_24 = f.__pfn; + f$__delta_5 = MEM[(struct *)&f]; + f$__pfn_24 = MEM[(struct *)&f + 4B]; ... Index: mine/gcc/testsuite/g++.dg/torture/pr44972.C =================================================================== --- /dev/null +++ mine/gcc/testsuite/g++.dg/torture/pr44972.C @@ -0,0 +1,142 @@ +/* { dg-do compile } */ + +#include<cassert> +#include<new> +#include<utility> + +namespace boost { + +template<class T> +class optional; + +class aligned_storage +{ + char data[ 1000 ]; + public: + void const* address() const { return &data[0]; } + void * address() { return &data[0]; } +} ; + + +template<class T> +class optional_base +{ + protected : + optional_base(){} + optional_base ( T const& val ) + { + construct(val); + } + + template<class U> + void assign ( optional<U> const& rhs ) + { + if (!is_initialized()) + if ( rhs.is_initialized() ) + construct(T()); + } + + public : + + bool is_initialized() const { return m_initialized ; } + + protected : + + void construct ( T const& val ) + { + new (m_storage.address()) T(val) ; + } + + T const* get_ptr_impl() const + { return static_cast<T const*>(m_storage.address()); } + + private : + + bool m_initialized ; + aligned_storage m_storage ; +} ; + + +template<class T> +class optional : public optional_base<T> +{ + typedef optional_base<T> base ; + + public : + + optional() : base() {} + optional ( T const& val ) : base(val) {} + optional& operator= ( optional const& rhs ) + { + this->assign( rhs ) ; + return *this ; + } + + T const& get() const ; + + T const* operator->() const { ((this->is_initialized()) ? static_cast<void> (0) : __assert_fail ("this->is_initialized()", "pr44972.C", 78, __PRETTY_FUNCTION__)) ; return this->get_ptr_impl() ; } + +} ; + + +} // namespace boost + + +namespace std +{ + + template<typename _Tp, std::size_t _Nm> + struct array + { + typedef _Tp value_type; + typedef const value_type* const_iterator; + + value_type _M_instance[_Nm]; + + }; +} + + +class NT +{ + double _inf, _sup; +}; + + +template < typename T > inline +std::array<T, 1> +make_array(const T& b1) +{ + std::array<T, 1> a = { { b1 } }; + return a; +} + +class V +{ + typedef std::array<NT, 1> Base; + Base base; + +public: + V() {} + V(const NT &x) + : base(make_array(x)) {} + +}; + +using boost::optional ; + +optional< std::pair< NT, NT > > + linsolve_pointC2() ; + +optional< V > construct_normal_offset_lines_isecC2 ( ) +{ + optional< std::pair<NT,NT> > ip; + + ip = linsolve_pointC2(); + + V a(ip->first) ; + return a; +} + + + ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Reimplementation of build_ref_for_offset 2010-09-08 17:19 ` [PATCH 2/2] Reimplementation of build_ref_for_offset Martin Jambor @ 2010-09-09 9:31 ` Richard Guenther 2010-09-09 22:44 ` Martin Jambor 2010-10-23 19:33 ` H.J. Lu 1 sibling, 1 reply; 11+ messages in thread From: Richard Guenther @ 2010-09-09 9:31 UTC (permalink / raw) To: Martin Jambor; +Cc: GCC Patches On Wed, 8 Sep 2010, Martin Jambor wrote: > Hi, > > this patch reimplements build_ref_for_offset so that it simply creates > a MEM_REF rather than trying to figure out what combination of > component and array refs are necessary. The main advantage of this > approach is that this can never fail, allowing us to be more > aggressive and remove a number of checks. > > There were two main problems with this, though. First is that > MEM_REFs are not particularly readable to by users. This would be a > problem when we are creating a reference that might be displayed to > them in a warning or a debugger which is what we do with > DECL_DEBUG_EXPR expressions. We sometimes construct these > artificially when propagating accesses across assignments. So for > those cases I retained the old implementation and only simplified it a > bit - it is now called build_user_friendly_ref_for_offset. > > The other problem was bit-fields. Constructing accesses to them was > difficult enough but then I realized that I was not even able to > detect the cases when I was accessing a bit field if their offset > happened to be on a byte boundary. I thought I would be able to > figure this out from TYPE_SIZE and TYPE_PRECISION of exp_type but > combinations that signal a bit-field in one language may not be > applied in another (in C, small TYPE_PRECISION denotes bit-fields and > TYPE_SIZE is big, but for example Fortran booleans have the precision > set to one even though they are not bit-fields). From types alone we can't see whether the object is part of a bitfield or not, only DECL_BIT_FIELD in FIELD_DECLs specify it. Thus recognizing and remembering this during access analysis is indeed the correct way of making sure to handle them correctly. Indeed MEM_REF will cause accesses of byte-granular size as specified by TYPE_SIZE_UNIT (TREE_TYPE (mem-ref)). While this might not be a problem for bitfield loads that are aligned to byte boundaries, bitfield stores will clobber whatever is adjacent to it and is not in the next byte(s). Until we lower all bit field accesses (I have plans to resurrect a pass doing that from the old mem-ref branch), bitfield accesses should be done by wrapping a BIT_FIELD_REF around generated MEM_REF trees. > So in the end I based the detection on the access structures that > represented the thing being loaded or stored which I knew had their > sizes correct because they are based on field sizes. Since I use the > access, the simplest way to actually create the reference to the bit > field is to re-use the last component ref of its expression - that is > what build_ref_for_model (meaning a model access) does. Separating > this from build_ref_for_offset (which cannot handle bit-fields) makes > the code a bit cleaner and keeps the latter function for other users > which know nothing about SRA access structures. > > I hope that you'll find these approaches reasonable. The patch was > bootstrapped and tested on x86_64-linux without any issues. I'd like > to commit it to trunk but I'm sure there will be comments and > suggestions. Comments inline. > Thanks, > > Martin > > > > 2010-09-08 Martin Jambor <mjambor@suse.cz> > > PR tree-optimization/44972 > * tree-sra.c: Include toplev.h. > (build_ref_for_offset): Entirely reimplemented. > (build_ref_for_model): New function. > (build_user_friendly_ref_for_offset): New function. > (analyze_access_subtree): Removed build_ref_for_offset check. > (propagate_subaccesses_across_link): Likewise. > (create_artificial_child_access): Use > build_user_friendly_ref_for_offset. > (propagate_subaccesses_across_link): Likewise. > (ref_expr_for_all_replacements_p): Removed. > (generate_subtree_copies): Updated comment. Use build_ref_for_model. > (sra_modify_expr): Use build_ref_for_model. > (load_assign_lhs_subreplacements): Likewise. > (sra_modify_assign): Removed ref_expr_for_all_replacements_p checks, > checks for return values of build_ref_for_offset. > * ipa-cp.c (ipcp_lattice_from_jfunc): No need to check return value of > build_ref_for_offset. > * ipa-prop.h: Include gimple.h > * ipa-prop.c (ipa_compute_jump_functions): Update to look for MEM_REFs. > (ipa_analyze_indirect_call_uses): Update comment. > * Makefile.in (tree-sra.o): Add $(GIMPLE_H) to dependencies. > (IPA_PROP_H): Likewise. > > * testsuite/gcc.dg/ipa/ipa-sra-1.c: Adjust scanning expressions. > * testsuite/gcc.dg/tree-ssa/pr45144.c: Likewise. > * testsuite/gcc.dg/tree-ssa/forwprop-5.c: Likewise and scan optimzed > dump instead. > * testsuite/g++.dg/torture/pr34850.C: Remove expected warning. > * gcc/testsuite/g++.dg/torture/pr44972.C: New test. > > Index: mine/gcc/tree-sra.c > =================================================================== > --- mine.orig/gcc/tree-sra.c > +++ mine/gcc/tree-sra.c > @@ -76,6 +76,7 @@ along with GCC; see the file COPYING3. > #include "coretypes.h" > #include "alloc-pool.h" > #include "tm.h" > +#include "toplev.h" > #include "tree.h" > #include "gimple.h" > #include "cgraph.h" > @@ -1320,15 +1321,114 @@ make_fancy_name (tree expr) > return XOBFINISH (&name_obstack, char *); > } > > -/* Helper function for build_ref_for_offset. > +/* Construct a MEM_REF that would reference a part of aggregate BASE of type > + EXP_TYPE at the given OFFSET. If BASE is something for which > + get_addr_base_and_unit_offset returns NULL, gsi must be non-NULL and is used > + to insert new statements either before or below the current one as specified > + by INSERT_AFTER. If offset is not aligned to bytes or EXP_TYPE is a > + bit_field. .. then? (The last sentence looks incomplete) This function is not capable of handling bitfields. */ > + > +tree > +build_ref_for_offset (tree base, HOST_WIDE_INT offset, > + tree exp_type, gimple_stmt_iterator *gsi, > + bool insert_after) > +{ > + tree prev_base = base; > + tree off; > + location_t loc = EXPR_LOCATION (base); > + HOST_WIDE_INT base_offset; > > - FIXME: Eventually this should be rewritten to either re-use the > - original access expression unshared (which is good for alias > - analysis) or to build a MEM_REF expression. */ > + gcc_checking_assert (offset % BITS_PER_UNIT == 0); > + > + base = get_addr_base_and_unit_offset (base, &base_offset); > + if (!base) > + { Can you explain when we get here? Can base be a reference with a variable offset like a[i]? I suppose it can't be a bit-field-ref, as on top of that we should never build another ref. > + gimple stmt; > + tree tmp, addr; > + > + gcc_checking_assert (gsi); > + tmp = create_tmp_reg (build_pointer_type (TREE_TYPE (prev_base)), NULL); > + add_referenced_var (tmp); > + tmp = make_ssa_name (tmp, NULL); > + addr = build_fold_addr_expr (unshare_expr (prev_base)); > + stmt = gimple_build_assign (tmp, addr); > + SSA_NAME_DEF_STMT (tmp) = stmt; > + if (insert_after) > + gsi_insert_after (gsi, stmt, GSI_NEW_STMT); > + else > + gsi_insert_before (gsi, stmt, GSI_SAME_STMT); > + I think you need to update_stmt (stmt) here. > + off = build_int_cst (reference_alias_ptr_type (prev_base), > + offset / BITS_PER_UNIT); > + base = tmp; > + } > + else if (TREE_CODE (base) == MEM_REF) > + { > + off = build_int_cst (TREE_TYPE (TREE_OPERAND (base, 1)), > + base_offset + offset / BITS_PER_UNIT); > + off = int_const_binop (PLUS_EXPR, TREE_OPERAND (base, 1), off, 0); > + base = unshare_expr (TREE_OPERAND (base, 0)); > + } > + else > + { > + off = build_int_cst (reference_alias_ptr_type (base), > + base_offset + offset / BITS_PER_UNIT); > + base = build_fold_addr_expr (unshare_expr (base)); > + } > + > + return fold_build2_loc (loc, MEM_REF, exp_type, base, off); > +} Ok sofar. > +/* Construct a memory reference to a part of an aggregate BASE at the given > + OFFSET and of the same type as MODEL. In case this is a reference to a > + bit-field, the function will replicate the last component_ref of model's > + expr to access it. GSI and INSERT_AFTER have the same meaning as in > + build_ref_for_offset. */ > + > +static tree > +build_ref_for_model (tree base, HOST_WIDE_INT offset, > + struct access *model, gimple_stmt_iterator *gsi, > + bool insert_after) > +{ > + tree t, exp_type; > + bool bitfield; > + > + if (offset % BITS_PER_UNIT != 0 > + || model->size < BITS_PER_UNIT > + || exact_log2 (model->size) == -1) > + { /* This access looks like a bitfield. */ > + gcc_checking_assert (TREE_CODE (model->expr) == COMPONENT_REF); If you have the COMPONENT_REF, why not do if (TREE_CODE (model->expr) == COMPONENT_REF && DECL_BIT_FIELD (TREE_OPERAND (model->expr, 1))) instead of the offset/size checks? That way it is 100% obvious. > + offset -= int_bit_position (TREE_OPERAND (model->expr, 1)); > + gcc_assert (offset % BITS_PER_UNIT == 0); We're doing that check in build_ref_for_offset. > + exp_type = TREE_TYPE (TREE_OPERAND (model->expr, 0)); > + bitfield = true; > + } > + else > + { > + exp_type = model->type; > + bitfield = false; > + } > + > + t = build_ref_for_offset (base, offset, exp_type, gsi, insert_after); Replicating this call in both arms above would make it easier to distinguish both cases. Which makes me wonder if in either of the case we can pass NULL for gsi? (relates to the question in build_ref_for_offset) > + if (bitfield) > + t = fold_build3_loc (EXPR_LOCATION (base), COMPONENT_REF, model->type, t, You use base location here, shouldn't this be model->expr location instead? If there are more callers to build_ref_for_offset I think it would make sense to pass the location to use for the built MEM_REF to it, rather than hard-coding the base location inside. And indeed a COMPONENT_REF also works (in fact it always works, unless a BIT_FIELD_REF which doesn't work on the LHS). Nice obvious idea ;) > + TREE_OPERAND (model->expr, 1), NULL_TREE); > + > + return t; > +} > + > +/* Construct a memory reference consisting of component_refs and array_refs to > + a part of an aggregate *RES (which is of type TYPE). The requested part > + should have type EXP_TYPE at be the given OFFSET. This function might not > + succeed, it returns true when it does and only then *RES points to something > + meaningful. This function should be used only to build expressions that we > + might need to present to user (e.g. in warnings). In all other situations, > + build_ref_for_model or build_ref_for_offset should be used instead. */ > > static bool > -build_ref_for_offset_1 (tree *res, tree type, HOST_WIDE_INT offset, > - tree exp_type) > +build_user_friendly_ref_for_offset (tree *res, tree type, HOST_WIDE_INT offset, > + tree exp_type) > { > while (1) > { > @@ -1367,19 +1467,13 @@ build_ref_for_offset_1 (tree *res, tree > else if (pos > offset || (pos + size) <= offset) > continue; > > - if (res) > + expr = build3 (COMPONENT_REF, TREE_TYPE (fld), *res, fld, > + NULL_TREE); > + expr_ptr = &expr; > + if (build_user_friendly_ref_for_offset (expr_ptr, TREE_TYPE (fld), > + offset - pos, exp_type)) > { > - expr = build3 (COMPONENT_REF, TREE_TYPE (fld), *res, fld, > - NULL_TREE); > - expr_ptr = &expr; > - } > - else > - expr_ptr = NULL; > - if (build_ref_for_offset_1 (expr_ptr, TREE_TYPE (fld), > - offset - pos, exp_type)) > - { > - if (res) > - *res = expr; > + *res = expr; > return true; > } > } > @@ -1394,14 +1488,11 @@ build_ref_for_offset_1 (tree *res, tree > minidx = TYPE_MIN_VALUE (TYPE_DOMAIN (type)); > if (TREE_CODE (minidx) != INTEGER_CST || el_size == 0) > return false; > - if (res) > - { > - index = build_int_cst (TYPE_DOMAIN (type), offset / el_size); > - if (!integer_zerop (minidx)) > - index = int_const_binop (PLUS_EXPR, index, minidx, 0); > - *res = build4 (ARRAY_REF, TREE_TYPE (type), *res, index, > - NULL_TREE, NULL_TREE); > - } > + index = build_int_cst (TYPE_DOMAIN (type), offset / el_size); > + if (!integer_zerop (minidx)) > + index = int_const_binop (PLUS_EXPR, index, minidx, 0); > + *res = build4 (ARRAY_REF, TREE_TYPE (type), *res, index, > + NULL_TREE, NULL_TREE); > offset = offset % el_size; > type = TREE_TYPE (type); > break; > @@ -1418,31 +1509,6 @@ build_ref_for_offset_1 (tree *res, tree > } > } Ok. I see the point about warnings, but when are MEM_REF trees displayed in a debugger? > -/* Construct an expression that would reference a part of aggregate *EXPR of > - type TYPE at the given OFFSET of the type EXP_TYPE. If EXPR is NULL, the > - function only determines whether it can build such a reference without > - actually doing it, otherwise, the tree it points to is unshared first and > - then used as a base for furhter sub-references. */ > - > -bool > -build_ref_for_offset (tree *expr, tree type, HOST_WIDE_INT offset, > - tree exp_type, bool allow_ptr) > -{ > - location_t loc = expr ? EXPR_LOCATION (*expr) : UNKNOWN_LOCATION; > - > - if (expr) > - *expr = unshare_expr (*expr); > - > - if (allow_ptr && POINTER_TYPE_P (type)) > - { > - type = TREE_TYPE (type); > - if (expr) > - *expr = build_simple_mem_ref_loc (loc, *expr); > - } > - > - return build_ref_for_offset_1 (expr, type, offset, exp_type); > -} > - > /* Return true iff TYPE is stdarg va_list type. */ > > static inline bool > @@ -1823,13 +1889,7 @@ analyze_access_subtree (struct access *r > > if (allow_replacements && scalar && !root->first_child > && (root->grp_hint > - || (root->grp_write && (direct_read || root->grp_assignment_read))) > - /* We must not ICE later on when trying to build an access to the > - original data within the aggregate even when it is impossible to do in > - a defined way like in the PR 42703 testcase. Therefore we check > - pre-emptively here that we will be able to do that. */ > - && build_ref_for_offset (NULL, TREE_TYPE (root->base), root->offset, > - root->type, false)) > + || (root->grp_write && (direct_read || root->grp_assignment_read)))) > { > if (dump_file && (dump_flags & TDF_DETAILS)) > { > @@ -1914,12 +1974,11 @@ create_artificial_child_access (struct a > { > struct access *access; > struct access **child; > - tree expr = parent->base;; > + tree expr = parent->base; > > gcc_assert (!model->grp_unscalarizable_region); > - > - if (!build_ref_for_offset (&expr, TREE_TYPE (expr), new_offset, > - model->type, false)) > + if (!build_user_friendly_ref_for_offset (&expr, TREE_TYPE (expr), new_offset, > + model->type)) > return NULL; Hm, so we don't fall back to creating a MEM_REF here? Or is this one case of relaxing checks that you want to postpone for later (which I think is fine)? > access = (struct access *) pool_alloc (access_pool); > @@ -1964,8 +2023,8 @@ propagate_subaccesses_across_link (struc > { > tree t = lacc->base; > > - if (build_ref_for_offset (&t, TREE_TYPE (t), lacc->offset, racc->type, > - false)) > + if (build_user_friendly_ref_for_offset (&t, TREE_TYPE (t), lacc->offset, > + racc->type)) > { > lacc->expr = t; > lacc->type = racc->type; > @@ -1994,13 +2053,6 @@ propagate_subaccesses_across_link (struc > continue; > } > > - /* If a (part of) a union field is on the RHS of an assignment, it can > - have sub-accesses which do not make sense on the LHS (PR 40351). > - Check that this is not the case. */ > - if (!build_ref_for_offset (NULL, TREE_TYPE (lacc->base), norm_offset, > - rchild->type, false)) > - continue; > - > rchild->grp_hint = 1; > new_acc = create_artificial_child_access (lacc, rchild, norm_offset); > if (new_acc) > @@ -2124,48 +2176,19 @@ analyze_all_variable_accesses (void) > return false; > } > > -/* Return true iff a reference statement into aggregate AGG can be built for > - every single to-be-replaced accesses that is a child of ACCESS, its sibling > - or a child of its sibling. TOP_OFFSET is the offset from the processed > - access subtree that has to be subtracted from offset of each access. */ > - > -static bool > -ref_expr_for_all_replacements_p (struct access *access, tree agg, > - HOST_WIDE_INT top_offset) > -{ > - do > - { > - if (access->grp_to_be_replaced > - && !build_ref_for_offset (NULL, TREE_TYPE (agg), > - access->offset - top_offset, > - access->type, false)) > - return false; > - > - if (access->first_child > - && !ref_expr_for_all_replacements_p (access->first_child, agg, > - top_offset)) > - return false; > - > - access = access->next_sibling; > - } > - while (access); > - > - return true; > -} > - > /* Generate statements copying scalar replacements of accesses within a subtree > into or out of AGG. ACCESS is the first child of the root of the subtree to > be processed. AGG is an aggregate type expression (can be a declaration but > - does not have to be, it can for example also be an indirect_ref). > - TOP_OFFSET is the offset of the processed subtree which has to be subtracted > - from offsets of individual accesses to get corresponding offsets for AGG. > - If CHUNK_SIZE is non-null, copy only replacements in the interval > - <start_offset, start_offset + chunk_size>, otherwise copy all. GSI is a > - statement iterator used to place the new statements. WRITE should be true > - when the statements should write from AGG to the replacement and false if > - vice versa. if INSERT_AFTER is true, new statements will be added after the > - current statement in GSI, they will be added before the statement > - otherwise. */ > + does not have to be, it can for example also be a mem_ref or a series of > + handled components). TOP_OFFSET is the offset of the processed subtree > + which has to be subtracted from offsets of individual accesses to get > + corresponding offsets for AGG. If CHUNK_SIZE is non-null, copy only > + replacements in the interval <start_offset, start_offset + chunk_size>, > + otherwise copy all. GSI is a statement iterator used to place the new > + statements. WRITE should be true when the statements should write from AGG > + to the replacement and false if vice versa. if INSERT_AFTER is true, new > + statements will be added after the current statement in GSI, they will be > + added before the statement otherwise. */ > > static void > generate_subtree_copies (struct access *access, tree agg, > @@ -2176,8 +2199,6 @@ generate_subtree_copies (struct access * > { > do > { > - tree expr = agg; > - > if (chunk_size && access->offset >= start_offset + chunk_size) > return; > > @@ -2185,14 +2206,11 @@ generate_subtree_copies (struct access * > && (chunk_size == 0 > || access->offset + access->size > start_offset)) > { > - tree repl = get_access_replacement (access); > - bool ref_found; > + tree expr, repl = get_access_replacement (access); > gimple stmt; > > - ref_found = build_ref_for_offset (&expr, TREE_TYPE (agg), > - access->offset - top_offset, > - access->type, false); > - gcc_assert (ref_found); > + expr = build_ref_for_model (agg, access->offset - top_offset, > + access, gsi, insert_after); > > if (write) > { > @@ -2329,12 +2347,10 @@ sra_modify_expr (tree *expr, gimple_stmt > in assembler statements (see PR42398). */ > if (!useless_type_conversion_p (type, access->type)) > { > - tree ref = access->base; > - bool ok; > + tree ref; > > - ok = build_ref_for_offset (&ref, TREE_TYPE (ref), > - access->offset, access->type, false); > - gcc_assert (ok); > + ref = build_ref_for_model (access->base, access->offset, access, > + NULL, false); > > if (write) > { > @@ -2458,25 +2474,11 @@ load_assign_lhs_subreplacements (struct > lhs, old_gsi); > > if (*refreshed == SRA_UDH_LEFT) > - { > - bool repl_found; > - > - rhs = lacc->base; > - repl_found = build_ref_for_offset (&rhs, TREE_TYPE (rhs), > - lacc->offset, lacc->type, > - false); > - gcc_assert (repl_found); > - } > + rhs = build_ref_for_model (lacc->base, lacc->offset, lacc, > + new_gsi, true); > else > - { > - bool repl_found; > - > - rhs = top_racc->base; > - repl_found = build_ref_for_offset (&rhs, > - TREE_TYPE (top_racc->base), > - offset, lacc->type, false); > - gcc_assert (repl_found); > - } > + rhs = build_ref_for_model (top_racc->base, offset, lacc, > + new_gsi, true); > } > > stmt = gimple_build_assign (get_access_replacement (lacc), rhs); > @@ -2633,25 +2635,18 @@ sra_modify_assign (gimple *stmt, gimple_ > if (AGGREGATE_TYPE_P (TREE_TYPE (lhs)) > && !access_has_children_p (lacc)) > { > - tree expr = lhs; > - if (build_ref_for_offset (&expr, TREE_TYPE (lhs), 0, > - TREE_TYPE (rhs), false)) > - { > - lhs = expr; > - gimple_assign_set_lhs (*stmt, expr); > - } > + lhs = build_ref_for_offset (lhs, 0, TREE_TYPE (rhs), gsi, false); > + gimple_assign_set_lhs (*stmt, lhs); > } > else if (AGGREGATE_TYPE_P (TREE_TYPE (rhs)) > + && !contains_view_convert_expr_p (rhs) > && !access_has_children_p (racc)) > - { > - tree expr = rhs; > - if (build_ref_for_offset (&expr, TREE_TYPE (rhs), 0, > - TREE_TYPE (lhs), false)) > - rhs = expr; > - } > + rhs = build_ref_for_offset (rhs, 0, TREE_TYPE (lhs), gsi, false); > + > if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs))) > { > - rhs = fold_build1_loc (loc, VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs); > + rhs = fold_build1_loc (loc, VIEW_CONVERT_EXPR, TREE_TYPE (lhs), > + rhs); > if (is_gimple_reg_type (TREE_TYPE (lhs)) > && TREE_CODE (lhs) != SSA_NAME) > force_gimple_rhs = true; > @@ -2694,11 +2689,7 @@ sra_modify_assign (gimple *stmt, gimple_ > > if (gimple_has_volatile_ops (*stmt) > || contains_view_convert_expr_p (rhs) > - || contains_view_convert_expr_p (lhs) > - || (access_has_children_p (racc) > - && !ref_expr_for_all_replacements_p (racc, lhs, racc->offset)) > - || (access_has_children_p (lacc) > - && !ref_expr_for_all_replacements_p (lacc, rhs, lacc->offset))) > + || contains_view_convert_expr_p (lhs)) > { > if (access_has_children_p (racc)) > generate_subtree_copies (racc->first_child, racc->base, 0, 0, 0, > Index: mine/gcc/ipa-cp.c > =================================================================== > --- mine.orig/gcc/ipa-cp.c > +++ mine/gcc/ipa-cp.c > @@ -327,7 +327,6 @@ ipcp_lattice_from_jfunc (struct ipa_node > { > struct ipcp_lattice *caller_lat; > tree t; > - bool ok; > > caller_lat = ipcp_get_lattice (info, jfunc->value.ancestor.formal_id); > lat->type = caller_lat->type; > @@ -340,16 +339,9 @@ ipcp_lattice_from_jfunc (struct ipa_node > return; > } > t = TREE_OPERAND (caller_lat->constant, 0); > - ok = build_ref_for_offset (&t, TREE_TYPE (t), > - jfunc->value.ancestor.offset, > - jfunc->value.ancestor.type, false); > - if (!ok) > - { > - lat->type = IPA_BOTTOM; > - lat->constant = NULL_TREE; > - } > - else > - lat->constant = build_fold_addr_expr (t); > + t = build_ref_for_offset (t, jfunc->value.ancestor.offset, > + jfunc->value.ancestor.type, NULL, false); > + lat->constant = build_fold_addr_expr (t); > } > else > lat->type = IPA_BOTTOM; > Index: mine/gcc/ipa-prop.h > =================================================================== > --- mine.orig/gcc/ipa-prop.h > +++ mine/gcc/ipa-prop.h > @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3. > #include "tree.h" > #include "vec.h" > #include "cgraph.h" > +#include "gimple.h" > > /* The following definitions and interfaces are used by > interprocedural analyses or parameters. */ > @@ -511,6 +512,7 @@ void ipa_prop_read_jump_functions (void) > void ipa_update_after_lto_read (void); > > /* From tree-sra.c: */ > -bool build_ref_for_offset (tree *, tree, HOST_WIDE_INT, tree, bool); > +tree build_ref_for_offset (tree, HOST_WIDE_INT, tree, gimple_stmt_iterator *, > + bool); > > #endif /* IPA_PROP_H */ > Index: mine/gcc/Makefile.in > =================================================================== > --- mine.orig/gcc/Makefile.in > +++ mine/gcc/Makefile.in > @@ -968,7 +968,7 @@ EBITMAP_H = ebitmap.h sbitmap.h > LTO_STREAMER_H = lto-streamer.h $(LINKER_PLUGIN_API_H) $(TARGET_H) \ > $(CGRAPH_H) $(VEC_H) vecprim.h $(TREE_H) $(GIMPLE_H) > TREE_VECTORIZER_H = tree-vectorizer.h $(TREE_DATA_REF_H) > -IPA_PROP_H = ipa-prop.h $(TREE_H) $(VEC_H) $(CGRAPH_H) > +IPA_PROP_H = ipa-prop.h $(TREE_H) $(VEC_H) $(CGRAPH_H) $(GIMPLE_H) > GSTAB_H = gstab.h stab.def > BITMAP_H = bitmap.h $(HASHTAB_H) statistics.h > GCC_PLUGIN_H = gcc-plugin.h highlev-plugin-common.h $(CONFIG_H) $(SYSTEM_H) \ > @@ -3142,10 +3142,10 @@ tree-ssa-ccp.o : tree-ssa-ccp.c $(TREE_F > tree-ssa-propagate.h value-prof.h $(FLAGS_H) $(TARGET_H) $(TOPLEV_H) $(DIAGNOSTIC_CORE_H) \ > $(DBGCNT_H) tree-pretty-print.h gimple-pretty-print.h > tree-sra.o : tree-sra.c $(CONFIG_H) $(SYSTEM_H) coretypes.h alloc-pool.h \ > - $(TM_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) $(IPA_PROP_H) \ > - $(DIAGNOSTIC_H) statistics.h $(TREE_DUMP_H) $(TIMEVAR_H) $(PARAMS_H) \ > - $(TARGET_H) $(FLAGS_H) $(EXPR_H) tree-pretty-print.h $(DBGCNT_H) \ > - $(TREE_INLINE_H) gimple-pretty-print.h > + $(TM_H) $(TOPLEV_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) \ > + $(IPA_PROP_H) $(DIAGNOSTIC_H) statistics.h $(TREE_DUMP_H) $(TIMEVAR_H) \ > + $(PARAMS_H) $(TARGET_H) $(FLAGS_H) $(EXPR_H) tree-pretty-print.h \ > + $(DBGCNT_H) $(TREE_INLINE_H) gimple-pretty-print.h > tree-switch-conversion.o : tree-switch-conversion.c $(CONFIG_H) $(SYSTEM_H) \ > $(TREE_H) $(TM_P_H) $(TREE_FLOW_H) $(DIAGNOSTIC_H) $(TREE_INLINE_H) \ > $(TIMEVAR_H) $(TM_H) coretypes.h $(TREE_DUMP_H) $(GIMPLE_H) \ > Index: mine/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c > =================================================================== > --- mine.orig/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c > +++ mine/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c > @@ -36,6 +36,5 @@ main (int argc, char *argv[]) > return 0; > } > > -/* { dg-final { scan-tree-dump "About to replace expr cow.green with ISRA" "eipa_sra" } } */ > -/* { dg-final { scan-tree-dump "About to replace expr cow.blue with ISRA" "eipa_sra" } } */ > +/* { dg-final { scan-tree-dump-times "About to replace expr" 2 "eipa_sra" } } */ > /* { dg-final { cleanup-tree-dump "eipa_sra" } } */ > Index: mine/gcc/testsuite/gcc.dg/tree-ssa/forwprop-5.c > =================================================================== > --- mine.orig/gcc/testsuite/gcc.dg/tree-ssa/forwprop-5.c > +++ mine/gcc/testsuite/gcc.dg/tree-ssa/forwprop-5.c > @@ -1,5 +1,5 @@ > /* { dg-do compile } */ > -/* { dg-options "-O1 -fdump-tree-esra -w" } */ > +/* { dg-options "-O1 -fdump-tree-optimized -w" } */ > > #define vector __attribute__((vector_size(16) )) > struct VecClass > @@ -11,12 +11,9 @@ vector float foo( vector float v ) > { > vector float x = v; > x = x + x; > - struct VecClass y = *(struct VecClass*)&x; > - return y.v; > + struct VecClass disappear = *(struct VecClass*)&x; > + return disappear.v; > } > > -/* We should be able to remove the intermediate struct and directly > - return x. As we do not fold VIEW_CONVERT_EXPR<struct VecClass>(x).v > - that doesn't happen right now. */ > -/* { dg-final { scan-tree-dump-times "VIEW_CONVERT_EXPR" 1 "esra"} } */ > -/* { dg-final { cleanup-tree-dump "esra" } } */ > +/* { dg-final { scan-tree-dump-times "disappear" 0 "optimized"} } */ > +/* { dg-final { cleanup-tree-dump "optimized" } } */ > Index: mine/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c > =================================================================== > --- mine.orig/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c > +++ mine/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c > @@ -42,5 +42,5 @@ bar (unsigned orig, unsigned *new) > *new = foo (&a); > } > > -/* { dg-final { scan-tree-dump "x = a;" "optimized"} } */ > +/* { dg-final { scan-tree-dump " = VIEW_CONVERT_EXPR<unsigned int>\\(a\\);" "optimized"} } */ Nice. > /* { dg-final { cleanup-tree-dump "optimized" } } */ > Index: mine/gcc/ipa-prop.c > =================================================================== > --- mine.orig/gcc/ipa-prop.c > +++ mine/gcc/ipa-prop.c > @@ -916,23 +916,27 @@ ipa_compute_jump_functions (struct cgrap > static tree > ipa_get_member_ptr_load_param (tree rhs, bool use_delta) > { > - tree rec, fld; > + tree rec, ref_offset, fld_offset; > tree ptr_field; > tree delta_field; > > - if (TREE_CODE (rhs) != COMPONENT_REF) > + if (TREE_CODE (rhs) != MEM_REF) > return NULL_TREE; Are you sure we never have a COMPONENT_REF here? We are not generally lowering them to MEM_REFs. The patch looks ok in general, minus my minor comments. Thanks, Richard. > - > rec = TREE_OPERAND (rhs, 0); > + if (TREE_CODE (rec) != ADDR_EXPR) > + return NULL_TREE; > + rec = TREE_OPERAND (rec, 0); > if (TREE_CODE (rec) != PARM_DECL > || !type_like_member_ptr_p (TREE_TYPE (rec), &ptr_field, &delta_field)) > return NULL_TREE; > > - fld = TREE_OPERAND (rhs, 1); > - if (use_delta ? (fld == delta_field) : (fld == ptr_field)) > - return rec; > + ref_offset = TREE_OPERAND (rhs, 1); > + if (use_delta) > + fld_offset = byte_position (delta_field); > else > - return NULL_TREE; > + fld_offset = byte_position (ptr_field); > + > + return tree_int_cst_equal (ref_offset, fld_offset) ? rec : NULL_TREE; > } > > /* If STMT looks like a statement loading a value from a member pointer formal > @@ -999,8 +1003,8 @@ ipa_note_param_call (struct cgraph_node > below, the call is on the last line: > > <bb 2>: > - f$__delta_5 = f.__delta; > - f$__pfn_24 = f.__pfn; > + f$__delta_5 = MEM[(struct *)&f]; > + f$__pfn_24 = MEM[(struct *)&f + 4B]; > > ... > > Index: mine/gcc/testsuite/g++.dg/torture/pr44972.C > =================================================================== > --- /dev/null > +++ mine/gcc/testsuite/g++.dg/torture/pr44972.C > @@ -0,0 +1,142 @@ > +/* { dg-do compile } */ > + > +#include<cassert> > +#include<new> > +#include<utility> > + > +namespace boost { > + > +template<class T> > +class optional; > + > +class aligned_storage > +{ > + char data[ 1000 ]; > + public: > + void const* address() const { return &data[0]; } > + void * address() { return &data[0]; } > +} ; > + > + > +template<class T> > +class optional_base > +{ > + protected : > + optional_base(){} > + optional_base ( T const& val ) > + { > + construct(val); > + } > + > + template<class U> > + void assign ( optional<U> const& rhs ) > + { > + if (!is_initialized()) > + if ( rhs.is_initialized() ) > + construct(T()); > + } > + > + public : > + > + bool is_initialized() const { return m_initialized ; } > + > + protected : > + > + void construct ( T const& val ) > + { > + new (m_storage.address()) T(val) ; > + } > + > + T const* get_ptr_impl() const > + { return static_cast<T const*>(m_storage.address()); } > + > + private : > + > + bool m_initialized ; > + aligned_storage m_storage ; > +} ; > + > + > +template<class T> > +class optional : public optional_base<T> > +{ > + typedef optional_base<T> base ; > + > + public : > + > + optional() : base() {} > + optional ( T const& val ) : base(val) {} > + optional& operator= ( optional const& rhs ) > + { > + this->assign( rhs ) ; > + return *this ; > + } > + > + T const& get() const ; > + > + T const* operator->() const { ((this->is_initialized()) ? static_cast<void> (0) : __assert_fail ("this->is_initialized()", "pr44972.C", 78, __PRETTY_FUNCTION__)) ; return this->get_ptr_impl() ; } > + > +} ; > + > + > +} // namespace boost > + > + > +namespace std > +{ > + > + template<typename _Tp, std::size_t _Nm> > + struct array > + { > + typedef _Tp value_type; > + typedef const value_type* const_iterator; > + > + value_type _M_instance[_Nm]; > + > + }; > +} > + > + > +class NT > +{ > + double _inf, _sup; > +}; > + > + > +template < typename T > inline > +std::array<T, 1> > +make_array(const T& b1) > +{ > + std::array<T, 1> a = { { b1 } }; > + return a; > +} > + > +class V > +{ > + typedef std::array<NT, 1> Base; > + Base base; > + > +public: > + V() {} > + V(const NT &x) > + : base(make_array(x)) {} > + > +}; > + > +using boost::optional ; > + > +optional< std::pair< NT, NT > > > + linsolve_pointC2() ; > + > +optional< V > construct_normal_offset_lines_isecC2 ( ) > +{ > + optional< std::pair<NT,NT> > ip; > + > + ip = linsolve_pointC2(); > + > + V a(ip->first) ; > + return a; > +} > + > + > + > > -- Richard Guenther <rguenther@suse.de> Novell / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Reimplementation of build_ref_for_offset 2010-09-09 9:31 ` Richard Guenther @ 2010-09-09 22:44 ` Martin Jambor 0 siblings, 0 replies; 11+ messages in thread From: Martin Jambor @ 2010-09-09 22:44 UTC (permalink / raw) To: Richard Guenther; +Cc: GCC Patches Hi, thank you very much for your comments. On Thu, Sep 09, 2010 at 11:29:29AM +0200, Richard Guenther wrote: > On Wed, 8 Sep 2010, Martin Jambor wrote: > > > 2010-09-08 Martin Jambor <mjambor@suse.cz> > > > > PR tree-optimization/44972 > > * tree-sra.c: Include toplev.h. > > (build_ref_for_offset): Entirely reimplemented. > > (build_ref_for_model): New function. > > (build_user_friendly_ref_for_offset): New function. > > (analyze_access_subtree): Removed build_ref_for_offset check. > > (propagate_subaccesses_across_link): Likewise. > > (create_artificial_child_access): Use > > build_user_friendly_ref_for_offset. > > (propagate_subaccesses_across_link): Likewise. > > (ref_expr_for_all_replacements_p): Removed. > > (generate_subtree_copies): Updated comment. Use build_ref_for_model. > > (sra_modify_expr): Use build_ref_for_model. > > (load_assign_lhs_subreplacements): Likewise. > > (sra_modify_assign): Removed ref_expr_for_all_replacements_p checks, > > checks for return values of build_ref_for_offset. > > * ipa-cp.c (ipcp_lattice_from_jfunc): No need to check return value of > > build_ref_for_offset. > > * ipa-prop.h: Include gimple.h > > * ipa-prop.c (ipa_compute_jump_functions): Update to look for MEM_REFs. > > (ipa_analyze_indirect_call_uses): Update comment. > > * Makefile.in (tree-sra.o): Add $(GIMPLE_H) to dependencies. > > (IPA_PROP_H): Likewise. > > > > * testsuite/gcc.dg/ipa/ipa-sra-1.c: Adjust scanning expressions. > > * testsuite/gcc.dg/tree-ssa/pr45144.c: Likewise. > > * testsuite/gcc.dg/tree-ssa/forwprop-5.c: Likewise and scan optimzed > > dump instead. > > * testsuite/g++.dg/torture/pr34850.C: Remove expected warning. > > * gcc/testsuite/g++.dg/torture/pr44972.C: New test. > > > > Index: mine/gcc/tree-sra.c > > =================================================================== > > --- mine.orig/gcc/tree-sra.c > > +++ mine/gcc/tree-sra.c > > @@ -76,6 +76,7 @@ along with GCC; see the file COPYING3. > > #include "coretypes.h" > > #include "alloc-pool.h" > > #include "tm.h" > > +#include "toplev.h" > > #include "tree.h" > > #include "gimple.h" > > #include "cgraph.h" > > @@ -1320,15 +1321,114 @@ make_fancy_name (tree expr) > > return XOBFINISH (&name_obstack, char *); > > } > > > > -/* Helper function for build_ref_for_offset. > > +/* Construct a MEM_REF that would reference a part of aggregate BASE of type > > + EXP_TYPE at the given OFFSET. If BASE is something for which > > + get_addr_base_and_unit_offset returns NULL, gsi must be non-NULL and is used > > + to insert new statements either before or below the current one as specified > > + by INSERT_AFTER. If offset is not aligned to bytes or EXP_TYPE is a > > + bit_field. > > .. then? (The last sentence looks incomplete) The whole sentence should have been removed, for some reason part of it wasn't. > > This function is not capable of handling bitfields. */ > > + > > +tree > > +build_ref_for_offset (tree base, HOST_WIDE_INT offset, > > + tree exp_type, gimple_stmt_iterator *gsi, > > + bool insert_after) > > +{ > > + tree prev_base = base; > > + tree off; > > + location_t loc = EXPR_LOCATION (base); > > + HOST_WIDE_INT base_offset; > > > > - FIXME: Eventually this should be rewritten to either re-use the > > - original access expression unshared (which is good for alias > > - analysis) or to build a MEM_REF expression. */ > > + gcc_checking_assert (offset % BITS_PER_UNIT == 0); > > + > > + base = get_addr_base_and_unit_offset (base, &base_offset); > > + if (!base) > > + { > > Can you explain when we get here? Can base be a reference > with a variable offset like a[i]? Yes, I have added a comment with this example. > I suppose it can't be a > bit-field-ref, as on top of that we should never build another > ref. No. I tried to think of a scenario when that could happen but no, for several reasons it can't. > > > + gimple stmt; > > + tree tmp, addr; > > + > > + gcc_checking_assert (gsi); > > + tmp = create_tmp_reg (build_pointer_type (TREE_TYPE (prev_base)), NULL); > > + add_referenced_var (tmp); > > + tmp = make_ssa_name (tmp, NULL); > > + addr = build_fold_addr_expr (unshare_expr (prev_base)); > > + stmt = gimple_build_assign (tmp, addr); > > + SSA_NAME_DEF_STMT (tmp) = stmt; > > + if (insert_after) > > + gsi_insert_after (gsi, stmt, GSI_NEW_STMT); > > + else > > + gsi_insert_before (gsi, stmt, GSI_SAME_STMT); > > + > > I think you need to update_stmt (stmt) here. Right. > > > + off = build_int_cst (reference_alias_ptr_type (prev_base), > > + offset / BITS_PER_UNIT); > > + base = tmp; > > + } > > + else if (TREE_CODE (base) == MEM_REF) > > + { > > + off = build_int_cst (TREE_TYPE (TREE_OPERAND (base, 1)), > > + base_offset + offset / BITS_PER_UNIT); > > + off = int_const_binop (PLUS_EXPR, TREE_OPERAND (base, 1), off, 0); > > + base = unshare_expr (TREE_OPERAND (base, 0)); > > + } > > + else > > + { > > + off = build_int_cst (reference_alias_ptr_type (base), > > + base_offset + offset / BITS_PER_UNIT); > > + base = build_fold_addr_expr (unshare_expr (base)); > > + } > > + > > + return fold_build2_loc (loc, MEM_REF, exp_type, base, off); > > +} > > Ok sofar. > > > +/* Construct a memory reference to a part of an aggregate BASE at the given > > + OFFSET and of the same type as MODEL. In case this is a reference to a > > + bit-field, the function will replicate the last component_ref of model's > > + expr to access it. GSI and INSERT_AFTER have the same meaning as in > > + build_ref_for_offset. */ > > + > > +static tree > > +build_ref_for_model (tree base, HOST_WIDE_INT offset, > > + struct access *model, gimple_stmt_iterator *gsi, > > + bool insert_after) > > +{ > > + tree t, exp_type; > > + bool bitfield; > > + > > + if (offset % BITS_PER_UNIT != 0 > > + || model->size < BITS_PER_UNIT > > + || exact_log2 (model->size) == -1) > > + { > /* This access looks like a bitfield. */ > > + gcc_checking_assert (TREE_CODE (model->expr) == COMPONENT_REF); > > If you have the COMPONENT_REF, why not do > > if (TREE_CODE (model->expr) == COMPONENT_REF > && DECL_BIT_FIELD (TREE_OPERAND (model->expr, 1))) > > instead of the offset/size checks? That way it is 100% obvious. > Yeah, this is much nicer. > > + offset -= int_bit_position (TREE_OPERAND (model->expr, 1)); > > + gcc_assert (offset % BITS_PER_UNIT == 0); > > We're doing that check in build_ref_for_offset. True, I removed this one. > > > + exp_type = TREE_TYPE (TREE_OPERAND (model->expr, 0)); > > + bitfield = true; > > + } > > + else > > + { > > + exp_type = model->type; > > + bitfield = false; > > + } > > + > > + t = build_ref_for_offset (base, offset, exp_type, gsi, insert_after); > > Replicating this call in both arms above would make it easier to > distinguish both cases. OK > Which makes me wonder if in either of > the case we can pass NULL for gsi? (relates to the question in > build_ref_for_offset) Yes, IPA-CP and inlining do that. This is safe because they always pass a base which is an is_gimple_ip_invariant - an address of (a part of) a global object. > > > + if (bitfield) > > + t = fold_build3_loc (EXPR_LOCATION (base), COMPONENT_REF, model->type, t, > > You use base location here, shouldn't this be model->expr location > instead? If there are more callers to build_ref_for_offset I think > it would make sense to pass the location to use for the built MEM_REF > to it, rather than hard-coding the base location inside. Yes, that is exactly my plan. Actually, I think most of the time the location of the expression or the statement that is currently being replaced/processed is what I'll pass down. I intend to look at all location handling in SRA (mainly because of PR 45505). I'll prepare a followup patch. > > ... > > @@ -1418,31 +1509,6 @@ build_ref_for_offset_1 (tree *res, tree > > } > > } > > Ok. I see the point about warnings, but when are MEM_REF trees > displayed in a debugger? Thinking about it again, they most certainly aren't. It was the warnings I was primarily concerned about. > > @@ -1823,13 +1889,7 @@ analyze_access_subtree (struct access *r > > > > if (allow_replacements && scalar && !root->first_child > > && (root->grp_hint > > - || (root->grp_write && (direct_read || root->grp_assignment_read))) > > - /* We must not ICE later on when trying to build an access to the > > - original data within the aggregate even when it is impossible to do in > > - a defined way like in the PR 42703 testcase. Therefore we check > > - pre-emptively here that we will be able to do that. */ > > - && build_ref_for_offset (NULL, TREE_TYPE (root->base), root->offset, > > - root->type, false)) > > + || (root->grp_write && (direct_read || root->grp_assignment_read)))) > > { > > if (dump_file && (dump_flags & TDF_DETAILS)) > > { > > @@ -1914,12 +1974,11 @@ create_artificial_child_access (struct a > > { > > struct access *access; > > struct access **child; > > - tree expr = parent->base;; > > + tree expr = parent->base; > > > > gcc_assert (!model->grp_unscalarizable_region); > > - > > - if (!build_ref_for_offset (&expr, TREE_TYPE (expr), new_offset, > > - model->type, false)) > > + if (!build_user_friendly_ref_for_offset (&expr, TREE_TYPE (expr), new_offset, > > + model->type)) > > return NULL; > > Hm, so we don't fall back to creating a MEM_REF here? Or is this > one case of relaxing checks that you want to postpone for later > (which I think is fine)? Yes, I'll need to add some flag to make sure any resulting replacements would have their TREE_NO_WARNING set. Most of the checks I'd like to go over and see whether we could do without them are those looking for V_C_Es. However, at the moment I'm having some problems bootstrapping Ada without them. ... > > Index: mine/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c > > =================================================================== > > --- mine.orig/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c > > +++ mine/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c > > @@ -42,5 +42,5 @@ bar (unsigned orig, unsigned *new) > > *new = foo (&a); > > } > > > > -/* { dg-final { scan-tree-dump "x = a;" "optimized"} } */ > > +/* { dg-final { scan-tree-dump " = VIEW_CONVERT_EXPR<unsigned int>\\(a\\);" "optimized"} } */ > > Nice. > > > /* { dg-final { cleanup-tree-dump "optimized" } } */ > > Index: mine/gcc/ipa-prop.c > > =================================================================== > > --- mine.orig/gcc/ipa-prop.c > > +++ mine/gcc/ipa-prop.c > > @@ -916,23 +916,27 @@ ipa_compute_jump_functions (struct cgrap > > static tree > > ipa_get_member_ptr_load_param (tree rhs, bool use_delta) > > { > > - tree rec, fld; > > + tree rec, ref_offset, fld_offset; > > tree ptr_field; > > tree delta_field; > > > > - if (TREE_CODE (rhs) != COMPONENT_REF) > > + if (TREE_CODE (rhs) != MEM_REF) > > return NULL_TREE; > > Are you sure we never have a COMPONENT_REF here? We are not > generally lowering them to MEM_REFs. The whole pattern matching relies on that ESRA has been run on the code (and I guess the subsequent early passes too), otherwise it would not work. So yes, I'm quite sure checking for MEM_REF is sufficient. > > The patch looks ok in general, minus my minor comments. > Thanks a lot, this is what I have just bootstrapped and tested on x86_64-linux and what I am going to commit. Martin 2010-09-09 Martin Jambor <mjambor@suse.cz> PR tree-optimization/44972 * tree-sra.c: Include toplev.h. (build_ref_for_offset): Entirely reimplemented. (build_ref_for_model): New function. (build_user_friendly_ref_for_offset): New function. (analyze_access_subtree): Removed build_ref_for_offset check. (propagate_subaccesses_across_link): Likewise. (create_artificial_child_access): Use build_user_friendly_ref_for_offset. (propagate_subaccesses_across_link): Likewise. (ref_expr_for_all_replacements_p): Removed. (generate_subtree_copies): Updated comment. Use build_ref_for_model. (sra_modify_expr): Use build_ref_for_model. (load_assign_lhs_subreplacements): Likewise. (sra_modify_assign): Removed ref_expr_for_all_replacements_p checks, checks for return values of build_ref_for_offset. * ipa-cp.c (ipcp_lattice_from_jfunc): No need to check return value of build_ref_for_offset. * ipa-prop.h: Include gimple.h * ipa-prop.c (ipa_compute_jump_functions): Update to look for MEM_REFs. (ipa_analyze_indirect_call_uses): Update comment. * Makefile.in (tree-sra.o): Add $(GIMPLE_H) to dependencies. (IPA_PROP_H): Likewise. * testsuite/gcc.dg/ipa/ipa-sra-1.c: Adjust scanning expressions. * testsuite/gcc.dg/tree-ssa/pr45144.c: Likewise. * testsuite/gcc.dg/tree-ssa/forwprop-5.c: Likewise and scan optimzed dump instead. * testsuite/g++.dg/torture/pr34850.C: Remove expected warning. * gcc/testsuite/g++.dg/torture/pr44972.C: New test. Index: mine/gcc/tree-sra.c =================================================================== --- mine.orig/gcc/tree-sra.c +++ mine/gcc/tree-sra.c @@ -76,6 +76,7 @@ along with GCC; see the file COPYING3. #include "coretypes.h" #include "alloc-pool.h" #include "tm.h" +#include "toplev.h" #include "tree.h" #include "gimple.h" #include "cgraph.h" @@ -1320,15 +1321,106 @@ make_fancy_name (tree expr) return XOBFINISH (&name_obstack, char *); } -/* Helper function for build_ref_for_offset. +/* Construct a MEM_REF that would reference a part of aggregate BASE of type + EXP_TYPE at the given OFFSET. If BASE is something for which + get_addr_base_and_unit_offset returns NULL, gsi must be non-NULL and is used + to insert new statements either before or below the current one as specified + by INSERT_AFTER. This function is not capable of handling bitfields. */ + +tree +build_ref_for_offset (tree base, HOST_WIDE_INT offset, + tree exp_type, gimple_stmt_iterator *gsi, + bool insert_after) +{ + tree prev_base = base; + tree off; + location_t loc = EXPR_LOCATION (base); + HOST_WIDE_INT base_offset; + + gcc_checking_assert (offset % BITS_PER_UNIT == 0); + + base = get_addr_base_and_unit_offset (base, &base_offset); + + /* get_addr_base_and_unit_offset returns NULL for references with a variable + offset such as array[var_index]. */ + if (!base) + { + gimple stmt; + tree tmp, addr; + + gcc_checking_assert (gsi); + tmp = create_tmp_reg (build_pointer_type (TREE_TYPE (prev_base)), NULL); + add_referenced_var (tmp); + tmp = make_ssa_name (tmp, NULL); + addr = build_fold_addr_expr (unshare_expr (prev_base)); + stmt = gimple_build_assign (tmp, addr); + SSA_NAME_DEF_STMT (tmp) = stmt; + if (insert_after) + gsi_insert_after (gsi, stmt, GSI_NEW_STMT); + else + gsi_insert_before (gsi, stmt, GSI_SAME_STMT); + update_stmt (stmt); + + off = build_int_cst (reference_alias_ptr_type (prev_base), + offset / BITS_PER_UNIT); + base = tmp; + } + else if (TREE_CODE (base) == MEM_REF) + { + off = build_int_cst (TREE_TYPE (TREE_OPERAND (base, 1)), + base_offset + offset / BITS_PER_UNIT); + off = int_const_binop (PLUS_EXPR, TREE_OPERAND (base, 1), off, 0); + base = unshare_expr (TREE_OPERAND (base, 0)); + } + else + { + off = build_int_cst (reference_alias_ptr_type (base), + base_offset + offset / BITS_PER_UNIT); + base = build_fold_addr_expr (unshare_expr (base)); + } + + return fold_build2_loc (loc, MEM_REF, exp_type, base, off); +} + +/* Construct a memory reference to a part of an aggregate BASE at the given + OFFSET and of the same type as MODEL. In case this is a reference to a + bit-field, the function will replicate the last component_ref of model's + expr to access it. GSI and INSERT_AFTER have the same meaning as in + build_ref_for_offset. */ + +static tree +build_ref_for_model (tree base, HOST_WIDE_INT offset, + struct access *model, gimple_stmt_iterator *gsi, + bool insert_after) +{ + if (TREE_CODE (model->expr) == COMPONENT_REF + && DECL_BIT_FIELD (TREE_OPERAND (model->expr, 1))) + { + /* This access represents a bit-field. */ + tree t, exp_type; + + offset -= int_bit_position (TREE_OPERAND (model->expr, 1)); + exp_type = TREE_TYPE (TREE_OPERAND (model->expr, 0)); + t = build_ref_for_offset (base, offset, exp_type, gsi, insert_after); + return fold_build3_loc (EXPR_LOCATION (base), COMPONENT_REF, + model->type, t, TREE_OPERAND (model->expr, 1), + NULL_TREE); + } + else + return build_ref_for_offset (base, offset, model->type, gsi, insert_after); +} - FIXME: Eventually this should be rewritten to either re-use the - original access expression unshared (which is good for alias - analysis) or to build a MEM_REF expression. */ +/* Construct a memory reference consisting of component_refs and array_refs to + a part of an aggregate *RES (which is of type TYPE). The requested part + should have type EXP_TYPE at be the given OFFSET. This function might not + succeed, it returns true when it does and only then *RES points to something + meaningful. This function should be used only to build expressions that we + might need to present to user (e.g. in warnings). In all other situations, + build_ref_for_model or build_ref_for_offset should be used instead. */ static bool -build_ref_for_offset_1 (tree *res, tree type, HOST_WIDE_INT offset, - tree exp_type) +build_user_friendly_ref_for_offset (tree *res, tree type, HOST_WIDE_INT offset, + tree exp_type) { while (1) { @@ -1367,19 +1459,13 @@ build_ref_for_offset_1 (tree *res, tree else if (pos > offset || (pos + size) <= offset) continue; - if (res) + expr = build3 (COMPONENT_REF, TREE_TYPE (fld), *res, fld, + NULL_TREE); + expr_ptr = &expr; + if (build_user_friendly_ref_for_offset (expr_ptr, TREE_TYPE (fld), + offset - pos, exp_type)) { - expr = build3 (COMPONENT_REF, TREE_TYPE (fld), *res, fld, - NULL_TREE); - expr_ptr = &expr; - } - else - expr_ptr = NULL; - if (build_ref_for_offset_1 (expr_ptr, TREE_TYPE (fld), - offset - pos, exp_type)) - { - if (res) - *res = expr; + *res = expr; return true; } } @@ -1394,14 +1480,11 @@ build_ref_for_offset_1 (tree *res, tree minidx = TYPE_MIN_VALUE (TYPE_DOMAIN (type)); if (TREE_CODE (minidx) != INTEGER_CST || el_size == 0) return false; - if (res) - { - index = build_int_cst (TYPE_DOMAIN (type), offset / el_size); - if (!integer_zerop (minidx)) - index = int_const_binop (PLUS_EXPR, index, minidx, 0); - *res = build4 (ARRAY_REF, TREE_TYPE (type), *res, index, - NULL_TREE, NULL_TREE); - } + index = build_int_cst (TYPE_DOMAIN (type), offset / el_size); + if (!integer_zerop (minidx)) + index = int_const_binop (PLUS_EXPR, index, minidx, 0); + *res = build4 (ARRAY_REF, TREE_TYPE (type), *res, index, + NULL_TREE, NULL_TREE); offset = offset % el_size; type = TREE_TYPE (type); break; @@ -1418,31 +1501,6 @@ build_ref_for_offset_1 (tree *res, tree } } -/* Construct an expression that would reference a part of aggregate *EXPR of - type TYPE at the given OFFSET of the type EXP_TYPE. If EXPR is NULL, the - function only determines whether it can build such a reference without - actually doing it, otherwise, the tree it points to is unshared first and - then used as a base for furhter sub-references. */ - -bool -build_ref_for_offset (tree *expr, tree type, HOST_WIDE_INT offset, - tree exp_type, bool allow_ptr) -{ - location_t loc = expr ? EXPR_LOCATION (*expr) : UNKNOWN_LOCATION; - - if (expr) - *expr = unshare_expr (*expr); - - if (allow_ptr && POINTER_TYPE_P (type)) - { - type = TREE_TYPE (type); - if (expr) - *expr = build_simple_mem_ref_loc (loc, *expr); - } - - return build_ref_for_offset_1 (expr, type, offset, exp_type); -} - /* Return true iff TYPE is stdarg va_list type. */ static inline bool @@ -1823,13 +1881,7 @@ analyze_access_subtree (struct access *r if (allow_replacements && scalar && !root->first_child && (root->grp_hint - || (root->grp_write && (direct_read || root->grp_assignment_read))) - /* We must not ICE later on when trying to build an access to the - original data within the aggregate even when it is impossible to do in - a defined way like in the PR 42703 testcase. Therefore we check - pre-emptively here that we will be able to do that. */ - && build_ref_for_offset (NULL, TREE_TYPE (root->base), root->offset, - root->type, false)) + || (root->grp_write && (direct_read || root->grp_assignment_read)))) { if (dump_file && (dump_flags & TDF_DETAILS)) { @@ -1914,12 +1966,11 @@ create_artificial_child_access (struct a { struct access *access; struct access **child; - tree expr = parent->base;; + tree expr = parent->base; gcc_assert (!model->grp_unscalarizable_region); - - if (!build_ref_for_offset (&expr, TREE_TYPE (expr), new_offset, - model->type, false)) + if (!build_user_friendly_ref_for_offset (&expr, TREE_TYPE (expr), new_offset, + model->type)) return NULL; access = (struct access *) pool_alloc (access_pool); @@ -1964,8 +2015,8 @@ propagate_subaccesses_across_link (struc { tree t = lacc->base; - if (build_ref_for_offset (&t, TREE_TYPE (t), lacc->offset, racc->type, - false)) + if (build_user_friendly_ref_for_offset (&t, TREE_TYPE (t), lacc->offset, + racc->type)) { lacc->expr = t; lacc->type = racc->type; @@ -1994,13 +2045,6 @@ propagate_subaccesses_across_link (struc continue; } - /* If a (part of) a union field is on the RHS of an assignment, it can - have sub-accesses which do not make sense on the LHS (PR 40351). - Check that this is not the case. */ - if (!build_ref_for_offset (NULL, TREE_TYPE (lacc->base), norm_offset, - rchild->type, false)) - continue; - rchild->grp_hint = 1; new_acc = create_artificial_child_access (lacc, rchild, norm_offset); if (new_acc) @@ -2124,48 +2168,19 @@ analyze_all_variable_accesses (void) return false; } -/* Return true iff a reference statement into aggregate AGG can be built for - every single to-be-replaced accesses that is a child of ACCESS, its sibling - or a child of its sibling. TOP_OFFSET is the offset from the processed - access subtree that has to be subtracted from offset of each access. */ - -static bool -ref_expr_for_all_replacements_p (struct access *access, tree agg, - HOST_WIDE_INT top_offset) -{ - do - { - if (access->grp_to_be_replaced - && !build_ref_for_offset (NULL, TREE_TYPE (agg), - access->offset - top_offset, - access->type, false)) - return false; - - if (access->first_child - && !ref_expr_for_all_replacements_p (access->first_child, agg, - top_offset)) - return false; - - access = access->next_sibling; - } - while (access); - - return true; -} - /* Generate statements copying scalar replacements of accesses within a subtree into or out of AGG. ACCESS is the first child of the root of the subtree to be processed. AGG is an aggregate type expression (can be a declaration but - does not have to be, it can for example also be an indirect_ref). - TOP_OFFSET is the offset of the processed subtree which has to be subtracted - from offsets of individual accesses to get corresponding offsets for AGG. - If CHUNK_SIZE is non-null, copy only replacements in the interval - <start_offset, start_offset + chunk_size>, otherwise copy all. GSI is a - statement iterator used to place the new statements. WRITE should be true - when the statements should write from AGG to the replacement and false if - vice versa. if INSERT_AFTER is true, new statements will be added after the - current statement in GSI, they will be added before the statement - otherwise. */ + does not have to be, it can for example also be a mem_ref or a series of + handled components). TOP_OFFSET is the offset of the processed subtree + which has to be subtracted from offsets of individual accesses to get + corresponding offsets for AGG. If CHUNK_SIZE is non-null, copy only + replacements in the interval <start_offset, start_offset + chunk_size>, + otherwise copy all. GSI is a statement iterator used to place the new + statements. WRITE should be true when the statements should write from AGG + to the replacement and false if vice versa. if INSERT_AFTER is true, new + statements will be added after the current statement in GSI, they will be + added before the statement otherwise. */ static void generate_subtree_copies (struct access *access, tree agg, @@ -2176,8 +2191,6 @@ generate_subtree_copies (struct access * { do { - tree expr = agg; - if (chunk_size && access->offset >= start_offset + chunk_size) return; @@ -2185,14 +2198,11 @@ generate_subtree_copies (struct access * && (chunk_size == 0 || access->offset + access->size > start_offset)) { - tree repl = get_access_replacement (access); - bool ref_found; + tree expr, repl = get_access_replacement (access); gimple stmt; - ref_found = build_ref_for_offset (&expr, TREE_TYPE (agg), - access->offset - top_offset, - access->type, false); - gcc_assert (ref_found); + expr = build_ref_for_model (agg, access->offset - top_offset, + access, gsi, insert_after); if (write) { @@ -2329,12 +2339,10 @@ sra_modify_expr (tree *expr, gimple_stmt in assembler statements (see PR42398). */ if (!useless_type_conversion_p (type, access->type)) { - tree ref = access->base; - bool ok; + tree ref; - ok = build_ref_for_offset (&ref, TREE_TYPE (ref), - access->offset, access->type, false); - gcc_assert (ok); + ref = build_ref_for_model (access->base, access->offset, access, + NULL, false); if (write) { @@ -2458,25 +2466,11 @@ load_assign_lhs_subreplacements (struct lhs, old_gsi); if (*refreshed == SRA_UDH_LEFT) - { - bool repl_found; - - rhs = lacc->base; - repl_found = build_ref_for_offset (&rhs, TREE_TYPE (rhs), - lacc->offset, lacc->type, - false); - gcc_assert (repl_found); - } + rhs = build_ref_for_model (lacc->base, lacc->offset, lacc, + new_gsi, true); else - { - bool repl_found; - - rhs = top_racc->base; - repl_found = build_ref_for_offset (&rhs, - TREE_TYPE (top_racc->base), - offset, lacc->type, false); - gcc_assert (repl_found); - } + rhs = build_ref_for_model (top_racc->base, offset, lacc, + new_gsi, true); } stmt = gimple_build_assign (get_access_replacement (lacc), rhs); @@ -2633,25 +2627,18 @@ sra_modify_assign (gimple *stmt, gimple_ if (AGGREGATE_TYPE_P (TREE_TYPE (lhs)) && !access_has_children_p (lacc)) { - tree expr = lhs; - if (build_ref_for_offset (&expr, TREE_TYPE (lhs), 0, - TREE_TYPE (rhs), false)) - { - lhs = expr; - gimple_assign_set_lhs (*stmt, expr); - } + lhs = build_ref_for_offset (lhs, 0, TREE_TYPE (rhs), gsi, false); + gimple_assign_set_lhs (*stmt, lhs); } else if (AGGREGATE_TYPE_P (TREE_TYPE (rhs)) + && !contains_view_convert_expr_p (rhs) && !access_has_children_p (racc)) - { - tree expr = rhs; - if (build_ref_for_offset (&expr, TREE_TYPE (rhs), 0, - TREE_TYPE (lhs), false)) - rhs = expr; - } + rhs = build_ref_for_offset (rhs, 0, TREE_TYPE (lhs), gsi, false); + if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs))) { - rhs = fold_build1_loc (loc, VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs); + rhs = fold_build1_loc (loc, VIEW_CONVERT_EXPR, TREE_TYPE (lhs), + rhs); if (is_gimple_reg_type (TREE_TYPE (lhs)) && TREE_CODE (lhs) != SSA_NAME) force_gimple_rhs = true; @@ -2694,11 +2681,7 @@ sra_modify_assign (gimple *stmt, gimple_ if (gimple_has_volatile_ops (*stmt) || contains_view_convert_expr_p (rhs) - || contains_view_convert_expr_p (lhs) - || (access_has_children_p (racc) - && !ref_expr_for_all_replacements_p (racc, lhs, racc->offset)) - || (access_has_children_p (lacc) - && !ref_expr_for_all_replacements_p (lacc, rhs, lacc->offset))) + || contains_view_convert_expr_p (lhs)) { if (access_has_children_p (racc)) generate_subtree_copies (racc->first_child, racc->base, 0, 0, 0, Index: mine/gcc/ipa-cp.c =================================================================== --- mine.orig/gcc/ipa-cp.c +++ mine/gcc/ipa-cp.c @@ -327,7 +327,6 @@ ipcp_lattice_from_jfunc (struct ipa_node { struct ipcp_lattice *caller_lat; tree t; - bool ok; caller_lat = ipcp_get_lattice (info, jfunc->value.ancestor.formal_id); lat->type = caller_lat->type; @@ -340,16 +339,9 @@ ipcp_lattice_from_jfunc (struct ipa_node return; } t = TREE_OPERAND (caller_lat->constant, 0); - ok = build_ref_for_offset (&t, TREE_TYPE (t), - jfunc->value.ancestor.offset, - jfunc->value.ancestor.type, false); - if (!ok) - { - lat->type = IPA_BOTTOM; - lat->constant = NULL_TREE; - } - else - lat->constant = build_fold_addr_expr (t); + t = build_ref_for_offset (t, jfunc->value.ancestor.offset, + jfunc->value.ancestor.type, NULL, false); + lat->constant = build_fold_addr_expr (t); } else lat->type = IPA_BOTTOM; Index: mine/gcc/ipa-prop.h =================================================================== --- mine.orig/gcc/ipa-prop.h +++ mine/gcc/ipa-prop.h @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3. #include "tree.h" #include "vec.h" #include "cgraph.h" +#include "gimple.h" /* The following definitions and interfaces are used by interprocedural analyses or parameters. */ @@ -511,6 +512,7 @@ void ipa_prop_read_jump_functions (void) void ipa_update_after_lto_read (void); /* From tree-sra.c: */ -bool build_ref_for_offset (tree *, tree, HOST_WIDE_INT, tree, bool); +tree build_ref_for_offset (tree, HOST_WIDE_INT, tree, gimple_stmt_iterator *, + bool); #endif /* IPA_PROP_H */ Index: mine/gcc/Makefile.in =================================================================== --- mine.orig/gcc/Makefile.in +++ mine/gcc/Makefile.in @@ -968,7 +968,7 @@ EBITMAP_H = ebitmap.h sbitmap.h LTO_STREAMER_H = lto-streamer.h $(LINKER_PLUGIN_API_H) $(TARGET_H) \ $(CGRAPH_H) $(VEC_H) vecprim.h $(TREE_H) $(GIMPLE_H) TREE_VECTORIZER_H = tree-vectorizer.h $(TREE_DATA_REF_H) -IPA_PROP_H = ipa-prop.h $(TREE_H) $(VEC_H) $(CGRAPH_H) +IPA_PROP_H = ipa-prop.h $(TREE_H) $(VEC_H) $(CGRAPH_H) $(GIMPLE_H) GSTAB_H = gstab.h stab.def BITMAP_H = bitmap.h $(HASHTAB_H) statistics.h GCC_PLUGIN_H = gcc-plugin.h highlev-plugin-common.h $(CONFIG_H) $(SYSTEM_H) \ @@ -3142,10 +3142,10 @@ tree-ssa-ccp.o : tree-ssa-ccp.c $(TREE_F tree-ssa-propagate.h value-prof.h $(FLAGS_H) $(TARGET_H) $(TOPLEV_H) $(DIAGNOSTIC_CORE_H) \ $(DBGCNT_H) tree-pretty-print.h gimple-pretty-print.h tree-sra.o : tree-sra.c $(CONFIG_H) $(SYSTEM_H) coretypes.h alloc-pool.h \ - $(TM_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) $(IPA_PROP_H) \ - $(DIAGNOSTIC_H) statistics.h $(TREE_DUMP_H) $(TIMEVAR_H) $(PARAMS_H) \ - $(TARGET_H) $(FLAGS_H) $(EXPR_H) tree-pretty-print.h $(DBGCNT_H) \ - $(TREE_INLINE_H) gimple-pretty-print.h + $(TM_H) $(TOPLEV_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) \ + $(IPA_PROP_H) $(DIAGNOSTIC_H) statistics.h $(TREE_DUMP_H) $(TIMEVAR_H) \ + $(PARAMS_H) $(TARGET_H) $(FLAGS_H) $(EXPR_H) tree-pretty-print.h \ + $(DBGCNT_H) $(TREE_INLINE_H) gimple-pretty-print.h tree-switch-conversion.o : tree-switch-conversion.c $(CONFIG_H) $(SYSTEM_H) \ $(TREE_H) $(TM_P_H) $(TREE_FLOW_H) $(DIAGNOSTIC_H) $(TREE_INLINE_H) \ $(TIMEVAR_H) $(TM_H) coretypes.h $(TREE_DUMP_H) $(GIMPLE_H) \ Index: mine/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c =================================================================== --- mine.orig/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c +++ mine/gcc/testsuite/gcc.dg/ipa/ipa-sra-1.c @@ -36,6 +36,5 @@ main (int argc, char *argv[]) return 0; } -/* { dg-final { scan-tree-dump "About to replace expr cow.green with ISRA" "eipa_sra" } } */ -/* { dg-final { scan-tree-dump "About to replace expr cow.blue with ISRA" "eipa_sra" } } */ +/* { dg-final { scan-tree-dump-times "About to replace expr" 2 "eipa_sra" } } */ /* { dg-final { cleanup-tree-dump "eipa_sra" } } */ Index: mine/gcc/testsuite/gcc.dg/tree-ssa/forwprop-5.c =================================================================== --- mine.orig/gcc/testsuite/gcc.dg/tree-ssa/forwprop-5.c +++ mine/gcc/testsuite/gcc.dg/tree-ssa/forwprop-5.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-O1 -fdump-tree-esra -w" } */ +/* { dg-options "-O1 -fdump-tree-optimized -w" } */ #define vector __attribute__((vector_size(16) )) struct VecClass @@ -11,12 +11,9 @@ vector float foo( vector float v ) { vector float x = v; x = x + x; - struct VecClass y = *(struct VecClass*)&x; - return y.v; + struct VecClass disappear = *(struct VecClass*)&x; + return disappear.v; } -/* We should be able to remove the intermediate struct and directly - return x. As we do not fold VIEW_CONVERT_EXPR<struct VecClass>(x).v - that doesn't happen right now. */ -/* { dg-final { scan-tree-dump-times "VIEW_CONVERT_EXPR" 1 "esra"} } */ -/* { dg-final { cleanup-tree-dump "esra" } } */ +/* { dg-final { scan-tree-dump-times "disappear" 0 "optimized"} } */ +/* { dg-final { cleanup-tree-dump "optimized" } } */ Index: mine/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c =================================================================== --- mine.orig/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c +++ mine/gcc/testsuite/gcc.dg/tree-ssa/pr45144.c @@ -42,5 +42,5 @@ bar (unsigned orig, unsigned *new) *new = foo (&a); } -/* { dg-final { scan-tree-dump "x = a;" "optimized"} } */ +/* { dg-final { scan-tree-dump " = VIEW_CONVERT_EXPR<unsigned int>\\(a\\);" "optimized"} } */ /* { dg-final { cleanup-tree-dump "optimized" } } */ Index: mine/gcc/ipa-prop.c =================================================================== --- mine.orig/gcc/ipa-prop.c +++ mine/gcc/ipa-prop.c @@ -916,23 +916,27 @@ ipa_compute_jump_functions (struct cgrap static tree ipa_get_member_ptr_load_param (tree rhs, bool use_delta) { - tree rec, fld; + tree rec, ref_offset, fld_offset; tree ptr_field; tree delta_field; - if (TREE_CODE (rhs) != COMPONENT_REF) + if (TREE_CODE (rhs) != MEM_REF) return NULL_TREE; - rec = TREE_OPERAND (rhs, 0); + if (TREE_CODE (rec) != ADDR_EXPR) + return NULL_TREE; + rec = TREE_OPERAND (rec, 0); if (TREE_CODE (rec) != PARM_DECL || !type_like_member_ptr_p (TREE_TYPE (rec), &ptr_field, &delta_field)) return NULL_TREE; - fld = TREE_OPERAND (rhs, 1); - if (use_delta ? (fld == delta_field) : (fld == ptr_field)) - return rec; + ref_offset = TREE_OPERAND (rhs, 1); + if (use_delta) + fld_offset = byte_position (delta_field); else - return NULL_TREE; + fld_offset = byte_position (ptr_field); + + return tree_int_cst_equal (ref_offset, fld_offset) ? rec : NULL_TREE; } /* If STMT looks like a statement loading a value from a member pointer formal @@ -999,8 +1003,8 @@ ipa_note_param_call (struct cgraph_node below, the call is on the last line: <bb 2>: - f$__delta_5 = f.__delta; - f$__pfn_24 = f.__pfn; + f$__delta_5 = MEM[(struct *)&f]; + f$__pfn_24 = MEM[(struct *)&f + 4B]; ... Index: mine/gcc/testsuite/g++.dg/torture/pr44972.C =================================================================== --- /dev/null +++ mine/gcc/testsuite/g++.dg/torture/pr44972.C @@ -0,0 +1,142 @@ +/* { dg-do compile } */ + +#include<cassert> +#include<new> +#include<utility> + +namespace boost { + +template<class T> +class optional; + +class aligned_storage +{ + char data[ 1000 ]; + public: + void const* address() const { return &data[0]; } + void * address() { return &data[0]; } +} ; + + +template<class T> +class optional_base +{ + protected : + optional_base(){} + optional_base ( T const& val ) + { + construct(val); + } + + template<class U> + void assign ( optional<U> const& rhs ) + { + if (!is_initialized()) + if ( rhs.is_initialized() ) + construct(T()); + } + + public : + + bool is_initialized() const { return m_initialized ; } + + protected : + + void construct ( T const& val ) + { + new (m_storage.address()) T(val) ; + } + + T const* get_ptr_impl() const + { return static_cast<T const*>(m_storage.address()); } + + private : + + bool m_initialized ; + aligned_storage m_storage ; +} ; + + +template<class T> +class optional : public optional_base<T> +{ + typedef optional_base<T> base ; + + public : + + optional() : base() {} + optional ( T const& val ) : base(val) {} + optional& operator= ( optional const& rhs ) + { + this->assign( rhs ) ; + return *this ; + } + + T const& get() const ; + + T const* operator->() const { ((this->is_initialized()) ? static_cast<void> (0) : __assert_fail ("this->is_initialized()", "pr44972.C", 78, __PRETTY_FUNCTION__)) ; return this->get_ptr_impl() ; } + +} ; + + +} // namespace boost + + +namespace std +{ + + template<typename _Tp, std::size_t _Nm> + struct array + { + typedef _Tp value_type; + typedef const value_type* const_iterator; + + value_type _M_instance[_Nm]; + + }; +} + + +class NT +{ + double _inf, _sup; +}; + + +template < typename T > inline +std::array<T, 1> +make_array(const T& b1) +{ + std::array<T, 1> a = { { b1 } }; + return a; +} + +class V +{ + typedef std::array<NT, 1> Base; + Base base; + +public: + V() {} + V(const NT &x) + : base(make_array(x)) {} + +}; + +using boost::optional ; + +optional< std::pair< NT, NT > > + linsolve_pointC2() ; + +optional< V > construct_normal_offset_lines_isecC2 ( ) +{ + optional< std::pair<NT,NT> > ip; + + ip = linsolve_pointC2(); + + V a(ip->first) ; + return a; +} + + + ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Reimplementation of build_ref_for_offset 2010-09-08 17:19 ` [PATCH 2/2] Reimplementation of build_ref_for_offset Martin Jambor 2010-09-09 9:31 ` Richard Guenther @ 2010-10-23 19:33 ` H.J. Lu 2011-05-18 15:58 ` H.J. Lu 1 sibling, 1 reply; 11+ messages in thread From: H.J. Lu @ 2010-10-23 19:33 UTC (permalink / raw) To: Martin Jambor; +Cc: GCC Patches, Richard Guenther On Wed, Sep 8, 2010 at 9:43 AM, Martin Jambor <mjambor@suse.cz> wrote: > Hi, > > this patch reimplements build_ref_for_offset so that it simply creates > a MEM_REF rather than trying to figure out what combination of > component and array refs are necessary. The main advantage of this > approach is that this can never fail, allowing us to be more > aggressive and remove a number of checks. > > There were two main problems with this, though. First is that > MEM_REFs are not particularly readable to by users. This would be a > problem when we are creating a reference that might be displayed to > them in a warning or a debugger which is what we do with > DECL_DEBUG_EXPR expressions. We sometimes construct these > artificially when propagating accesses across assignments. So for > those cases I retained the old implementation and only simplified it a > bit - it is now called build_user_friendly_ref_for_offset. > > The other problem was bit-fields. Constructing accesses to them was > difficult enough but then I realized that I was not even able to > detect the cases when I was accessing a bit field if their offset > happened to be on a byte boundary. I thought I would be able to > figure this out from TYPE_SIZE and TYPE_PRECISION of exp_type but > combinations that signal a bit-field in one language may not be > applied in another (in C, small TYPE_PRECISION denotes bit-fields and > TYPE_SIZE is big, but for example Fortran booleans have the precision > set to one even though they are not bit-fields). > > So in the end I based the detection on the access structures that > represented the thing being loaded or stored which I knew had their > sizes correct because they are based on field sizes. Since I use the > access, the simplest way to actually create the reference to the bit > field is to re-use the last component ref of its expression - that is > what build_ref_for_model (meaning a model access) does. Separating > this from build_ref_for_offset (which cannot handle bit-fields) makes > the code a bit cleaner and keeps the latter function for other users > which know nothing about SRA access structures. > > I hope that you'll find these approaches reasonable. The patch was > bootstrapped and tested on x86_64-linux without any issues. I'd like > to commit it to trunk but I'm sure there will be comments and > suggestions. > > Thanks, > > Martin > > > > 2010-09-08 Martin Jambor <mjambor@suse.cz> > > PR tree-optimization/44972 > * tree-sra.c: Include toplev.h. > (build_ref_for_offset): Entirely reimplemented. > (build_ref_for_model): New function. > (build_user_friendly_ref_for_offset): New function. > (analyze_access_subtree): Removed build_ref_for_offset check. > (propagate_subaccesses_across_link): Likewise. > (create_artificial_child_access): Use > build_user_friendly_ref_for_offset. > (propagate_subaccesses_across_link): Likewise. > (ref_expr_for_all_replacements_p): Removed. > (generate_subtree_copies): Updated comment. Use build_ref_for_model. > (sra_modify_expr): Use build_ref_for_model. > (load_assign_lhs_subreplacements): Likewise. > (sra_modify_assign): Removed ref_expr_for_all_replacements_p checks, > checks for return values of build_ref_for_offset. > * ipa-cp.c (ipcp_lattice_from_jfunc): No need to check return value of > build_ref_for_offset. > * ipa-prop.h: Include gimple.h > * ipa-prop.c (ipa_compute_jump_functions): Update to look for MEM_REFs. > (ipa_analyze_indirect_call_uses): Update comment. > * Makefile.in (tree-sra.o): Add $(GIMPLE_H) to dependencies. > (IPA_PROP_H): Likewise. This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46150 H.J. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Reimplementation of build_ref_for_offset 2010-10-23 19:33 ` H.J. Lu @ 2011-05-18 15:58 ` H.J. Lu 0 siblings, 0 replies; 11+ messages in thread From: H.J. Lu @ 2011-05-18 15:58 UTC (permalink / raw) To: Martin Jambor; +Cc: GCC Patches, Richard Guenther On Sat, Oct 23, 2010 at 10:12 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Sep 8, 2010 at 9:43 AM, Martin Jambor <mjambor@suse.cz> wrote: >> Hi, >> >> this patch reimplements build_ref_for_offset so that it simply creates >> a MEM_REF rather than trying to figure out what combination of >> component and array refs are necessary. The main advantage of this >> approach is that this can never fail, allowing us to be more >> aggressive and remove a number of checks. >> >> There were two main problems with this, though. First is that >> MEM_REFs are not particularly readable to by users. This would be a >> problem when we are creating a reference that might be displayed to >> them in a warning or a debugger which is what we do with >> DECL_DEBUG_EXPR expressions. We sometimes construct these >> artificially when propagating accesses across assignments. So for >> those cases I retained the old implementation and only simplified it a >> bit - it is now called build_user_friendly_ref_for_offset. >> >> The other problem was bit-fields. Constructing accesses to them was >> difficult enough but then I realized that I was not even able to >> detect the cases when I was accessing a bit field if their offset >> happened to be on a byte boundary. I thought I would be able to >> figure this out from TYPE_SIZE and TYPE_PRECISION of exp_type but >> combinations that signal a bit-field in one language may not be >> applied in another (in C, small TYPE_PRECISION denotes bit-fields and >> TYPE_SIZE is big, but for example Fortran booleans have the precision >> set to one even though they are not bit-fields). >> >> So in the end I based the detection on the access structures that >> represented the thing being loaded or stored which I knew had their >> sizes correct because they are based on field sizes. Since I use the >> access, the simplest way to actually create the reference to the bit >> field is to re-use the last component ref of its expression - that is >> what build_ref_for_model (meaning a model access) does. Separating >> this from build_ref_for_offset (which cannot handle bit-fields) makes >> the code a bit cleaner and keeps the latter function for other users >> which know nothing about SRA access structures. >> >> I hope that you'll find these approaches reasonable. The patch was >> bootstrapped and tested on x86_64-linux without any issues. I'd like >> to commit it to trunk but I'm sure there will be comments and >> suggestions. >> >> Thanks, >> >> Martin >> >> >> >> 2010-09-08 Martin Jambor <mjambor@suse.cz> >> >> PR tree-optimization/44972 >> * tree-sra.c: Include toplev.h. >> (build_ref_for_offset): Entirely reimplemented. >> (build_ref_for_model): New function. >> (build_user_friendly_ref_for_offset): New function. >> (analyze_access_subtree): Removed build_ref_for_offset check. >> (propagate_subaccesses_across_link): Likewise. >> (create_artificial_child_access): Use >> build_user_friendly_ref_for_offset. >> (propagate_subaccesses_across_link): Likewise. >> (ref_expr_for_all_replacements_p): Removed. >> (generate_subtree_copies): Updated comment. Use build_ref_for_model. >> (sra_modify_expr): Use build_ref_for_model. >> (load_assign_lhs_subreplacements): Likewise. >> (sra_modify_assign): Removed ref_expr_for_all_replacements_p checks, >> checks for return values of build_ref_for_offset. >> * ipa-cp.c (ipcp_lattice_from_jfunc): No need to check return value of >> build_ref_for_offset. >> * ipa-prop.h: Include gimple.h >> * ipa-prop.c (ipa_compute_jump_functions): Update to look for MEM_REFs. >> (ipa_analyze_indirect_call_uses): Update comment. >> * Makefile.in (tree-sra.o): Add $(GIMPLE_H) to dependencies. >> (IPA_PROP_H): Likewise. > > This caused: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46150 > This also caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49039 -- H.J. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-05-18 14:36 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-09-08 17:16 [PATCH 0/2] Fix PR 44972 Martin Jambor 2010-09-08 17:16 ` [PATCH 1/2] Remove the IPA-SRA call to build_ref_for_offset Martin Jambor 2010-09-09 9:01 ` Richard Guenther 2010-09-11 2:09 ` H.J. Lu 2010-12-06 18:56 ` H.J. Lu 2011-01-13 23:59 ` H.J. Lu 2010-09-08 17:19 ` [PATCH 2/2] Reimplementation of build_ref_for_offset Martin Jambor 2010-09-09 9:31 ` Richard Guenther 2010-09-09 22:44 ` Martin Jambor 2010-10-23 19:33 ` H.J. Lu 2011-05-18 15:58 ` H.J. Lu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).