Re: [RFC][patch][PR102281]Clear padding for variables that are in registers

[Qing Zhao <qing.zhao@oracle.com>]

On Oct 14, 2021, at 4:06 AM, Jakub Jelinek <jakub@redhat.com> wrote:

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.

Okay, I see now.

Then during gimplification phase, I will add “is_gimple_reg” to decide whether adding call to __builtin_clear_padding or not.


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).

With the current implementation,  for a long double/_Complex long double auto variable, if it is not explicitly initialized, -ftrivial-auto-var-init will add a call to .DEFERRED_INIT during gimplification, and expand this call as block initialization including its padding during RTL expansion;  No any issue when the variable is NOT explicitly initialized.

Only when it is explicitly initialized, and when it will be spilled into memory during RTL, its padding might not be initialized. Therefore It's necessary to clear its padding during RTL phase when this variable is spilled to memory.

It is tricky to fix this issue, and it’s not needed to fix PR102281, I will create another PR  to record this long double/_Complex lang double padding initialization issue and fix it later.


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,

You mean the following is not efficient enough:

/* Return true if TYPE contains any padding bits.  */

bool
clear_padding_type_has_padding_p (tree type)
{
 bool has_padding = false;
 if (BITS_PER_UNIT == 8
     && CHAR_BIT == 8
     && clear_padding_type_may_have_padding_p (type))
   {
     HOST_WIDE_INT sz = int_size_in_bytes (type), i;
     gcc_assert (sz > 0);
     unsigned char *buf = XALLOCAVEC (unsigned char, sz);
     memset (buf, ~0, sz);
     clear_type_padding_in_mask (type, buf);
     for (i = 0; i < sz; i++)
     if (buf[i] != (unsigned char) ~0)
       {
         has_padding = true;
         break;
       }
   }
 return has_padding;
}


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.

So, what’s the difference between “will __builtin_clear_padding emit any code on this type” and “any padding is found for this type”?
Should the answers to these two questions be the same?

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.

Is it convenient for you to modify the code in gimple-fold.c and send me the patch on a more efficient way to implement the new routine
clear_padding_type_has_padding_p?

Thanks a lot for your help.

Qing




Jakub