This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi Richard and Bernhard. > -----Original Message----- > From: Richard Biener [mailto:richard.guenther@gmail.com] > Sent: Tuesday, November 10, 2015 5:33 PM > To: Kumar, Venkataramanan > Cc: Andrew Pinski; gcc-patches@gcc.gnu.org > Subject: Re: [RFC] [PATCH V2]: RE: [RFC] [Patch] Relax tree-if-conv.c trap > assumptions. > > On Sat, Nov 7, 2015 at 12:41 PM, Kumar, Venkataramanan > <Venkataramanan.Kumar@amd.com> wrote: > > Hi Richard, > > > > I have now implemented storing of DR and references using hash maps. > > Please find attached patch. > > > > As discussed, I am now storing the ref, DR and baseref, DR pairs along with > unconditional read/write information in hash tables while iterating over DR > during its initialization. > > Then during checking for possible traps for if-converting, just check if the > memory reference for a gimple statement is read/written unconditionally by > querying the hash table instead of quadratic walk. > > > > Boot strapped and regression tested on x86_64. > > @@ -592,137 +598,153 @@ struct ifc_dr { > > /* -1 when not initialized, 0 when false, 1 when true. */ > int rw_unconditionally; > + > + tree ored_result; > + > > excess vertical space at the end. A better name would be simply "predicate". > > + if (!exsist1) > + { > + if (is_true_predicate (ca)) > + { > + DR_RW_UNCONDITIONALLY (a) = 1; > + } > > - while (TREE_CODE (ref_base_a) == COMPONENT_REF > - || TREE_CODE (ref_base_a) == IMAGPART_EXPR > - || TREE_CODE (ref_base_a) == REALPART_EXPR) > - ref_base_a = TREE_OPERAND (ref_base_a, 0); > + IFC_DR (a)->ored_result = ca; > + *master_dr = a; > + } > + else > + { > + IFC_DR (*master_dr)->ored_result > + = fold_or_predicates > + (EXPR_LOCATION (IFC_DR (*master_dr)->ored_result), > + ca, IFC_DR (*master_dr)->ored_result); > > - while (TREE_CODE (ref_base_b) == COMPONENT_REF > - || TREE_CODE (ref_base_b) == IMAGPART_EXPR > - || TREE_CODE (ref_base_b) == REALPART_EXPR) > - ref_base_b = TREE_OPERAND (ref_base_b, 0); > + if (is_true_predicate (ca) > + || is_true_predicate (IFC_DR (*master_dr)->ored_result)) > + { > + DR_RW_UNCONDITIONALLY (*master_dr) = 1; > + } > + } > > please common the predicate handling from both cases, that is, do > > if (is_true_predicate (IFC_DR (*master_dr)->ored_result)) > DR_RW_... > > after the if. Note no {}s around single stmts. > > Likewise for the base_master_dr code. > > + if (!found) > + { > + DR_WRITTEN_AT_LEAST_ONCE (a) =0; > + DR_RW_UNCONDITIONALLY (a) = 0; > + return false; > > no need to update the flags on non-"master" DRs. > > Please remove the 'found' variable and simply return 'true' > whenever found. > > static bool > ifcvt_memrefs_wont_trap (gimple *stmt, vec<data_reference_p> refs) { > - return write_memrefs_written_at_least_once (stmt, refs) > - && memrefs_read_or_written_unconditionally (stmt, refs); > + return memrefs_read_or_written_unconditionally (stmt, refs); > > please simply inline memrefs_read_or_written_unconditionally here. > > + if (ref_DR_map) > + delete ref_DR_map; > + ref_DR_map = NULL; > + > + if (baseref_DR_map) > + delete baseref_DR_map; > + baseref_DR_map = NULL; > > at this point the pointers will never be NULL. > > Ok with those changes. The attached patch addresses all the review comments and is rebased to current trunk. GCC regression test and bootstrap passed. Wanted to check with you once before committing it to trunk. Ok for trunk? gcc/ChangeLog 2015-11-15 Venkataramanan <Venkataramanan.Kumar@amd.com> * tree-if-conv.c (ref_DR_map): Define. (baseref_DR_map): Like wise (struct ifc_dr): Add new tree predicate field. (hash_memrefs_baserefs_and_store_DRs_read_written_info): New function. (memrefs_read_or_written_unconditionally): Use hash maps to query unconditional read/written information. (write_memrefs_written_at_least_once): Remove. (ifcvt_memrefs_wont_trap): Remove call to write_memrefs_written_at_least_once. (if_convertible_loop_p_1): Initialize hash maps and predicates before hashing data references. * tree-data-ref.h (decl_binds_to_current_def_p): Declare . gcc/testsuite/ChangeLog 2015-11-15 Venkataramanan <Venkataramanan.Kumar@amd.com> * gcc.dg/tree-ssa/ifc-8.c: Add new. Regards, Venkat. > > Note the tree-hash-traits.h part is already committed. > > > > gcc/ChangeLog > > 2015-11-07 Venkataramanan <Venkataramanan.Kumar@amd.com> > > > > *tree-hash-traits.h (struct tree_operand_hash). Use compare_type > and value_type. > > (equal_keys): Rename to equal and use compare_type and > value_type. > > * tree-if-conv.c (ref_DR_map): Define. > > (baseref_DR_map): Like wise > > (struct ifc_dr): New tree predicate field. > > (hash_memrefs_baserefs_and_store_DRs_read_written_info): New > function. > > (memrefs_read_or_written_unconditionally): Use hashmap to query > unconditional read/written information. > > (write_memrefs_written_at_least_once) : Remove. > > (ifcvt_memrefs_wont_trap): Remove call to > write_memrefs_written_at_least_once. > > (if_convertible_loop_p_1): Initialize/delete hash maps an initialize > predicates > > before the call to > hash_memrefs_baserefs_and_store_DRs_read_written_info. > > Watch changelog formatting. > > Thanks, > Richard. > > > gcc/testsuite/ChangeLog > > 2015-11-07 Venkataramanan <Venkataramanan.Kumar@amd.com> > > *gcc.dg/tree-ssa/ifc-8.c: Add new. > > > > Ok for trunk? > > > > Regards, > > Venkat. > >> > >> >> > + } > >> >> > + } > >> >> > + } > >> >> > + return found; > >> >> > +} > >> >> > + > >> >> > /* Return true when the memory references of STMT won't trap in > the > >> >> > if-converted code. There are two things that we have to check for: > >> >> > > >> >> > @@ -748,8 +776,20 @@ write_memrefs_written_at_least_once > >> (gimple > >> >> > *stmt, static bool ifcvt_memrefs_wont_trap (gimple *stmt, > >> >> > vec<data_reference_p> refs) { > >> >> > - return write_memrefs_written_at_least_once (stmt, refs) > >> >> > - && memrefs_read_or_written_unconditionally (stmt, refs); > >> >> > + bool memrefs_written_once, > >> memrefs_read_written_unconditionally; > >> >> > + bool memrefs_safe_to_access; > >> >> > + > >> >> > + memrefs_written_once > >> >> > + = write_memrefs_written_at_least_once (stmt, > >> >> > + refs); > >> >> > + > >> >> > + memrefs_read_written_unconditionally > >> >> > + = memrefs_read_or_written_unconditionally (stmt, > >> >> > + refs); > >> >> > + > >> >> > + memrefs_safe_to_access > >> >> > + = write_memrefs_safe_to_access_unconditionally > >> >> > + (stmt, refs); > >> >> > + > >> >> > + return ((memrefs_written_once || memrefs_safe_to_access) > >> >> > + && memrefs_read_written_unconditionally); > >> >> > } > >> >> > > >> >> > /* Wrapper around gimple_could_trap_p refined for the needs of > >> >> > the > >> >> > > >> >> > > >> >> > do I need this function write_memrefs_written_at_least_once > >> anymore? > >> >> > Please suggest if there a better way to do this. > >> >> > > >> >> > Bootstrapped and regression tested on x86_64. > >> >> > I am not adding change log and comments now, as I wanted to > >> >> > check > >> >> approach first. > >> >> > > >> >> > Regards, > >> >> > Venkat. > >> >> > > >> >> > > >
Attachment:
relax-trap-assumptions-V3.txt
Description: relax-trap-assumptions-V3.txt
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |