On Tue, Oct 12, 2021 at 10:51:45PM +0000, Qing Zhao wrote:
> PR10228 1https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102281
> Exposed an issue in the current implementation of the padding initialization
> of -ftrivial-auto-var-init.
>
> Currently, we add __builtin_clear_padding call _AFTER_ every explicit
> initialization of an auto variable:
>
> var_decl = {init_constructor};
> __builtin_clear_padding (&var_decl, 0B, 1);
>
> the reason I added the call to EVERY auto variable that has explicit
> initialization is, the folding of __builtin_clear_padding will automatically
> turn this call to a NOP when there is no padding in the variable. So, we
> don't need to check whether the variable has padding explicitly.
>
> However, always adding the call to __builtin_clear_padding (&var_decl,…)
> might introduce invalid IR when VAR_DECL cannot be addressable.
>
> In order to resolve this issue, I propose the following solution:
>
> Instead of adding the call to __builtin_clear_padding _AFTER_ the explicit
> initialization, Using zero to initialize the whole variable BEFORE explicit
> fields initialization when VAR_DECL has padding, i.e:
>
> If (had_padding_p (var_decl))
> var_decl = ZERO;
> var_decl = {init_constructor};
>
> This should resolve the invalid IR issue. However, there might be more run
> time overhead from such padding initialization since the whole variable is
> set to zero instead of only the paddings.
>
> Please let me know you comments on this.
As I've tried to explain in the PR, this is wrong.
It makes no sense whatsoever to clear padding on is_gimple_reg
variables even if they do have padding (e.g. long double/_Complex long
double), because all the GIMPLE passes will just not care about it.
If you want to clear padding for those, it needs to be done where it
is forced into memory (e.g. expansion for returning or passing such
types in memory if they aren't passed in registers, or RA when the register
allocator decides to spill them into memory if even that is what you want to
cover).
For !is_gimple_reg vars, yes, something like clear_padding_type_has_padding_p
could be useful to avoid unnecessary IL growth, but it should be implemented
more efficiently, in particular for the answer to a question will
__builtin_clear_padding emit any code on this type at least for non-unions
there is no buffer covering the whole type needed, just next to
clear_in_mask bool add another bool for the new mode and in clear_padding_flush
in that mode just set another bool if clear_in_mask mode would clear any bit
in the mask if it was there. For unions, I'm afraid it needs to do it
the clear_in_mask way and at the end analyze the buf.
Also, besides answer to the question "would __builtin_clear_padding emit any
code on this type" we should have another function/another mode which
returns true if any padding whatsoever is found, including unions.
For non-union it would be exactly the same thing, but in unions it would
keep using the same mode too, even if just one union member has any padding
return it. And, for these modes, there could be shortcuts, once we find
some padding, it doesn't make sense to walk further fields.
Jakub
