On Mon, Sep 25, 2017 at 11:35 AM, Eric Botcazou <ebotca...@adacore.com> wrote:
>> ISTR SUBREG_PROMOTED_* gets set by RTL expansion, the problematic
>> one should be the one in expr.c setting it on all SSA_NAMEs, maybe it is
>> enough to avoid setting it for SSA_NAME_IS_DEFAULT_DEF ones?
>
> No, it's not enough in this case, you need to work a little harder and look at
> the arguments of a PHI node because the SSA_NAME_IS_DEFAULT_DEF name is never
> materialized in the RTL; so the attached patch works in this case.
>
>
> * expr.c (expand_expr_real_1) <expand_decl_rtl>: For a SSA_NAME, do
> not set SUBREG_PROMOTED_VAR_P on the sub-register if it may contain
> uninitialized bits.
Reading the patch I think that it gets conservativeness wrong -- shouldn't it be
if (is_definitely_initialized)
{
SUBREG_PROMOTED_VAR_P (temp) = 1;
SUBREG_PROMOTED_SET (temp, unsignedp);
}
? Of course it's not easy to compute is_definitely_initialized conservatively
in an ad-hoc way at this place. It should be relatively straight-forward to
do a conservative computation somewhere in cfgexpand.c by propagating
across SSA edges and recording a flag on SSA names though. I assume
we can take load destinations as fully initialized (should extend properly)
as well as call results (the ABI should extend, eventually we can query
the target if it does), likewise for function arguments.
On your patch:
+ /* Try to detect if the register contains uninitialized bits. */
+ if (SSA_NAME_IS_DEFAULT_DEF (ssa_name))
+ maybe_uninitialized = true;
if you use ssa_undefined_value_p (ssa_name[, true]) you'd get function
paramters not undefined (which is probably desired?). Likewise
partial initialized complex would get uninitialized (if passed , true).
Same inside the loop over PHI args though I wonder how pessimizing
it would be to simply do
if (ssa_undefined_value_p (ssa_name, true)
|| gimple_code (SSA_NAME_DEF_STMT (ssa_name)) == GIMPLE_PHI)
maybe_uninitialized = true;
thus make all PHIs possibly uninitialized (your code wouldn't catch a
chained PHI with undef arg).
As said, a better solution would be to do a definitely-initialized
mini-propagation
at RTL expansion time.
I don't stand in the way of "improving" things with your patch (besides the
ssa_undefined_value_p use).
Thanks,
Richard.
> --
> Eric Botcazou
