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.
I'd say you should split out the base_predicate introduction into a
separate patch
(this change looks ok).
Richard.
> Regards,
> Venkat.
>
