[patch] Perform anonymous constant propagation during inlining

[Richard Biener]

On May 1, 2015 12:27:17 PM GMT+02:00, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> 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.

Yeah, I think that's a way better place for the hack.

Richard.