Re: [RFC] New SRA and early interprocedural SRA

[Ralf Wildenhues <Ralf dot Wildenhues at gmx dot de>]

Hello Martin,

typo-only nits (including typos in identifiers):

* Martin Jambor wrote on Wed, Mar 04, 2009 at 12:08:54PM CET:
> Index: isra/gcc/ipa-sra.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ isra/gcc/ipa-sra.c	2009-03-04 01:00:30.000000000 +0100

> +
> +   3. The pass sorts all accesses for a particular parameter and searches for
> +      any overlaps (a pair of accesses which both cover a part of an agregate

aggregate

> +      but at least one also covers a part not covered by the other).  If there

> +   4. The pass then modifies the parameters in both declaration and the type of
> +      the current function.  Afterwards it traverses the function again,
> +      replacing all references to components of the reduced parameters by the
> +      new parameters, possibly removing an indirect_ref and so on.  Finally, it
> +      converts all callers so that they pass the new required parameters and
> +      makes sure the function is private (ie. not COMDAT).

i.e.,

> +/* Return true iff TYPE is one of the types we consider aggregate for SRA
> +   tranformations.  */

transformations

> +static bool
> +sra_aggregate_type_p (tree type)
> +{
> +  return (TREE_CODE (type) == UNION_TYPE || TREE_CODE (type) == RECORD_TYPE
> +	  || TREE_CODE (type) == ARRAY_TYPE
> +	  || TREE_CODE (type) == QUAL_UNION_TYPE);
> +}
> +
> +/* Return true iff the type contains a field or element type which dpoes not

does

> +   allow scalarization.  */
> +
> +static bool
> +type_internals_preclude_sra_p (tree type)

> +/* If T is an SSA_NAME, return NULL if it is not a default def or return its
> +   base variable if it is.  Retrn T if it is not an SSA_NAME.  */

Return

> +static tree
> +get_ssa_base_param (tree t)

> +/* Result code for scan_assign callback for scan_function.  */
> +enum scan_assign_result {SRA_SA_NONE,       /* nothing done for the stmt */
> +			 SRA_SA_PROCESSED,  /* stmt analyzed/changed */
> +			 SRA_SA_REMOVED};   /* stmt redundant and eliminated */
> +
> +
> +/* Scan expressions occuring in the statement pointed to by STMT_EXPR, create

occurring

> +   access structures for all accesses to candidates for scalarization and
> +   remove those candidates which occur in statements or expressions that
> +   prevent them from being split apart.  Return true if any access has been
> +   inserted.  */
> +
> +static enum scan_assign_result
> +build_accesses_from_assign (gimple *stmt_ptr,
> +			    gimple_stmt_iterator *gsi ATTRIBUTE_UNUSED,
> +			    void *data)

> +/* Return the representative access for the parameter declaration PARM if it is
> +   a scalar passed by reference which is not written to and the pointer value
> +   is not used directly.  Thus, if it is legal to dereference it in the caller
> +   and we can rule out modifications through aliases, such parametershould be

'parameters should' or 'a parameter should'

> +   turned into one passed by value.  Return NULL otherwise.  */
> +
> +static struct access *
> +unmodified_by_ref_scalar_representative (tree parm)

> +/* Analyze the collected accesses and produce a plan what to do with the
> +   parameters in the form of notes, NULL meaning nothing.  */
> +
> +static VEC (new_parm_note_t, heap) *
> +analyze_all_param_acesses (void)

analyze_all_param_accesses, please.


> +/* Perform early interprocedural SRA.  */
> +
> +static unsigned int
> +ipa_early_sra (void)
> +{
> +  struct cgraph_node *node = cgraph_node (current_function_decl);
> +  VEC (new_parm_note_t, heap) *notes;
> +  int ret = 0;
> +
> +  /* DECL_WEAK test is required because otherwise we might get $symbolis

$symbols?

> +     already defined assembler messages (as we e.g. get for libgomp
> +     collapse-2.exe test case (at -O1).  */

> +	  if (dump_file)
> +	    {
> +	      fprintf (dump_file, "Inhibitingly overlapping access: ");

Inhibiting?

> +	      dump_access (access, false);
> +	    }
> +	  return NULL;
> +	}

> +enum build_access_tree_result
> +  {
> +    SRA_BAT_NONE,                 /* nothing scalarized */
> +    SRA_BAT_SCALAR_COMPONENTS,	  /* there are scalarized subcomponents but the
> +				     subtree is not fully covered with them  */
> +    SRA_BAT_SCALAR_COVERADGE	  /* the whole subtree covered by scalar

SRA_BAT_SCALAR_COVERAGE, please.

> +				     replacements  */
> +  };

> +/* Assign zero to all scalart replacements in an access subtree.  ACCESS is the

scalar

> +   the root of the subtree to be processed.  GSI is the statement iterator used
> +   for inserting statements which are added after the curent statement if

current

> +   INSERT_AFTER is true or before it otherwise.  */
> +static void
> +init_subtree_with_zero (struct access *access, gimple_stmt_iterator *gsi,
> +			bool insert_after)

> +static enum scan_assign_result
> +sra_intra_modify_constructor_assign (gimple *stmt, gimple_stmt_iterator *gsi)

> +  if (VEC_length (constructor_elt,
> +		  CONSTRUCTOR_ELTS (gimple_assign_rhs1 (*stmt))) > 0)
> +    {
> +      /* I have never seen this code path trigger but if it can happen the
> +	 following should hadle it gracefully.  */

handle

> +      if (acc && acc->first_child)
> +	generate_subtree_copies (acc->first_child, acc->base, 0, 0, 0, gsi,
> +				 true, true);
> +      return SRA_SA_PROCESSED;
> +    }

> +/* Callback of scan_function to process assign statements.  It examines both
> +   sides of the statement, replaces them with a scalare replacement if there is

scalar

> +   one and generating copying of replacements if scalarized aggregates have been

generates copies of?

> +   used in the assignment.  STMT is a pointer to the assign statement, GSI is
> +   used to hold generated statements for type conversions and subtree
> +   copying.  */
> +
> +static enum scan_assign_result
> +sra_intra_modify_assign (gimple *stmt, gimple_stmt_iterator *gsi,
> +			 void *data ATTRIBUTE_UNUSED)

Cheers,
Ralf