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: [patch] Perform anonymous constant propagation during inlining


> Hmm, special-casing this in the inliner looks odd to me.  ISTR the inliner
> already propagates constant parameters to immediate uses, so I guess
> you run into the casting case you handle specially.

Right on both counts, the original GIMPLE looks like:

  right.3 = (system__storage_elements__integer_address) right;
  D.4203 = D.4201 % right.3;
  D.4200 = (system__storage_elements__storage_offset) D.4203;
  return D.4200;

and setup_one_parameter has:

  /* If there is no setup required and we are in SSA, take the easy route
     replacing all SSA names representing the function parameter by the
     SSA name passed to function.

     We need to construct map for the variable anyway as it might be used
     in different SSA names when parameter is set in function.

     Do replacement at -O0 for const arguments replaced by constant.
     This is important for builtin_constant_p and other construct requiring
     constant argument to be visible in inlined function body.  */
  if (gimple_in_ssa_p (cfun) && rhs && def && is_gimple_reg (p)
      && (optimize
          || (TREE_READONLY (p)
	      && is_gimple_min_invariant (rhs)))
      && (TREE_CODE (rhs) == SSA_NAME
	  || is_gimple_min_invariant (rhs))
      && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (def))
    {
      insert_decl_map (id, def, rhs);
      return insert_init_debug_bind (id, bb, var, rhs, NULL);
    }

and here is the GIMPLE after inlining:

  _17 = _16;
  _18 = _17;
  right.3_19 = 8;
  _20 = _18 % right.3_19;
  _21 = (system__storage_elements__storage_offset) _20;

so the constant replacement is done for right.3_19 by the above block.

> But then if any constant propagation is ok at -O0 why not perform
> full-fledged constant propagation (with !DECL_IGNORED_P vars as a
> propagation barrier) as a regular SSA pass?  That is, if you'd had a
> Inline_Always function like
> 
> int foo (int i)
> {
>   return (i + 1) / i;
> }
> 
> you'd not get the desired result as the i + 1 stmt wouldn't be folded
> and thus the (i + 1) / i stmt wouldn't either.

That's OK, only the trivial cases need to be dealt with since it's -O0 so 
running a fully-fledged constant propagation seems overkill.

> That said - why does RTL optimization not save you here anyway?
> After all, it is responsible for turning divisions into shifts.

You mean the RTL expander?  Because the SSA name isn't replaced with the RHS 
of its defining statement in:

      /* For EXPAND_INITIALIZER try harder to get something simpler.  */
      if (g == NULL
	  && modifier == EXPAND_INITIALIZER
	  && !SSA_NAME_IS_DEFAULT_DEF (exp)
	  && (optimize || DECL_IGNORED_P (SSA_NAME_VAR (exp)))
	  && stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp)))
	g = SSA_NAME_DEF_STMT (exp);

since modifier is EXPAND_NORMAL.  Do you think it would be OK to be a little 
more aggressive here?  Something like:

      /* If we aren't on the LHS, look into the defining statement.  */
      if (g == NULL
	  && modifier != EXPAND_WRITE
	  && !SSA_NAME_IS_DEFAULT_DEF (exp)
	  && (optimize || DECL_IGNORED_P (SSA_NAME_VAR (exp)))
	  && stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp)))
	 {
	    g = SSA_NAME_DEF_STMT (exp);
	    /* For EXPAND_INITIALIZER substitute in all cases, otherwise do
	       it only for constants.  */
	    if (modifier != EXPAND_INITIALIZER
		&& (!gimple_assign_copy_p (g)
		    || !is_gimple_min_invariant (gimple_assign_rhs1 (g))))
	      g = NULL;
	 }

That's certainly sufficient here.

-- 
Eric Botcazou


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]