On Fri, Nov 27, 2015 at 9:24 AM, Kumar, Venkataramanan <venkataramanan.ku...@amd.com> wrote: > Hi Richard, > >> -----Original Message----- >> From: Richard Biener [mailto:richard.guent...@gmail.com] >> Sent: Tuesday, November 24, 2015 9:07 PM >> To: Kumar, Venkataramanan >> Cc: Jakub Jelinek (ja...@redhat.com); gcc-patches@gcc.gnu.org >> Subject: Re: [RFC] [Patch] PR67326 - relax trap assumption by looking at >> similar DRS >> >> On Fri, Nov 20, 2015 at 1:02 PM, Kumar, Venkataramanan >> <venkataramanan.ku...@amd.com> wrote: >> > Hi Richard, >> > >> > As per Jakub suggestion in >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67326, the below patch fixes >> the regression in tree if conversion. >> > Basically allowing if conversion to happen for a candidate DR, if we find >> similar DR with same dimensions and that DR will not trap. >> > >> > To find similar DRs using hash table to hashing the offset and DR pairs. >> > Also reusing read/written information that was stored for reference tree. >> > >> > Also. >> > (1) I guard these checks for -ftree-loop-if-convert-stores and -fno- >> common. >> > Sometimes vectorization flags also triggers if conversion. >> > (2) Also hashing base DRs for writes only. >> > >> > gcc/ChangeLog >> > 2015-11-19 Venkataramanan <venkataramanan.ku...@amd.com> >> > >> > PR tree-optimization/67326 >> > * tree-if-conv.c (offset_DR_map): Define. >> > (struct ifc_dr): Add new tree base_predicate field. >> > (hash_memrefs_baserefs_and_store_DRs_read_written_info): Hash >> offsets, DR pairs >> > and hash base ref, DR pairs for write type DRs. >> > (ifcvt_memrefs_wont_trap): Guard checks with -ftree-loop-if- >> convert-stores flag. >> > Check for similar DR that are accessed unconditionally. >> > (if_convertible_loop_p_1): Initialize and delete offset hash >> > maps >> > >> > gcc/testsuite/ChangeLog >> > 2015-11-19 Venkataramanan <venkataramanan.ku...@amd.com> >> > * gcc.dg/tree-ssa/ifc-pr67326.c: Add new. >> > >> > Regstrapped on x86_64, Ok for trunk? >> >> + if (offset) >> + { >> + offset_master_dr = &offset_DR_map->get_or_insert (offset,&exist3); >> + if (!exist3) >> + *offset_master_dr = a; >> + >> + if (DR_RW_UNCONDITIONALLY (*offset_master_dr) != 1) >> + DR_RW_UNCONDITIONALLY (*offset_master_dr) >> + = DR_RW_UNCONDITIONALLY (*master_dr); >> >> this is fishy - as far as I can see offset_master globs all _candidates_ and >> >> + else if (DR_OFFSET (a)) >> + { >> + offset_dr = offset_DR_map->get (DR_OFFSET (a)); >> + if ((DR_RW_UNCONDITIONALLY (*offset_dr) == 1) >> + && DR_NUM_DIMENSIONS (a) == DR_NUM_DIMENSIONS >> (*offset_dr)) >> + { >> + tree base_tree = get_base_address (DR_REF (a)); >> + if (DECL_P (base_tree) >> + && flag_tree_loop_if_convert_stores >> + && decl_binds_to_current_def_p (base_tree) >> + && !TREE_READONLY (base_tree)) >> + return true; >> + } >> + } >> >> where with this that actually checks something (DR_NUM_DIMENSIONS is >> not something you can use to identify two arrays with the same domain) will >> then consider DR_DW_UNCONDITIONALLY ORed from all _candidates_ but >> not only from those which really have the same domain. >> >> You need to do the domain check as part of the hash-map >> hashing/comparing. >> >> Note that there is no bounds info in the data ref info so you need to >> a) consider DR_OFFSET + DR_INIT >> b) verify the access size is the same (TYPE_SIZE_UNIT (TREE_TYPE (dr- >> >ref))) >> c) verify the base objects are of the same size - note this is somewhat >> difficult as the base object for DR_OFFSET/INIT is starting at >> DR_BASE_ADDRESS so maybe restrict this to ADDR_EXPR <decl> >> DR_BASE_ADDRESS cases where you can look at DECL_SIZE (decl) of both >> candidates >> >> You can also try using indices (DR_BASE_OBJECT plus DR_ACCESS_FNS when >> DR_UNCONSTRAINED_BASE is false). If the size of DR_BASE_OBJECT >> matches and all access functions are equal it should be a compatible enough >> case as well. > > Ok, I will take some time to figure out on domain analysis part. > >> >> I'd say you should split out the base_predicate introduction into a separate >> patch (this change looks ok). >> > > Attached patch has the "base_predicate" introduction part alone. > It does the predicate folding and hashes base references for only write type > DRs while hashing. > I have not added any new test case since we already have ifc-8.c > > Also fixed formatting issues Jakub pointed out for this patch. > > Boot strapped on X86_64. > > Ok to upstream if it passes regression tests?
Ok. Thanks, Richard. > gcc/ChangeLog > 2015-11-27 Venkataramanan Kumar <venkataramanan.ku...@amd.com> > > * tree-if-conv.c (struct ifc_dr): Add new tree > base_predicate field. > (hash_memrefs_baserefs_and_store_DRs_read_written_info): Hash > base ref, DR pairs and store base_predicate for write type DRs. > (ifcvt_memrefs_wont_trap): Guard checks with > -ftree-loop-if-convert-stores flag > > Regards, > Venkat
