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]

RE: [RFC] [PATCH V2]: RE: [RFC] [Patch] Relax tree-if-conv.c trap assumptions.


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]