[Bug middle-end/107411] trivial-auto-var-init=zero invalid uninitialized variable warning

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107411

--- Comment #8 from Qing Zhao  ---
> On Feb 16, 2023, at 2:35 AM, rguenther at suse dot de 
>  wrote:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107411
> 
> --- Comment #7 from rguenther at suse dot de  ---
> On Wed, 15 Feb 2023, qinzhao at gcc dot gnu.org wrote:
> 
> 
> Hmm, I don't think so.  So this is indeed expected behavior since the
> frontend IL doesn't have variable definitions with initializers but
> instead just (immediately following) assignments.

Then, if that’s the case, it also is correct to add the .DEFERRED_INIT to them
during gimplification?

[Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107952

--- Comment #15 from Qing Zhao  ---
> On Jan 25, 2023, at 11:12 AM, siddhesh at gcc dot gnu.org 
>  wrote:
>> 
>>> The first is handled by the function just fine,
>> 
>> No, even the first case is not recognized by the current
>> “array_ref_flexible_size_p”, it’s not been identified as a flexible array
>> right now.
>> Shall we include this case into “array_ref_flexible_size_p”?  (It’s a GCC
>> extension).
> 
> In the first case, array_ref_flexible_size_p recognizes S2.flex.data as having
> flexible size.  The tests in my patch[1] for this bug checks for this.
Oh, yes. That’s right.
> 
> However, array_ref_flexible_size_p does not recognize S2.flex as having
> flexible size.  It might make sense to support that, i.e. any struct or union
> with the last element as a flex array should be recognized as having flexible
> size.

Since S2.flex is not an “array_ref”, it’s correct for array_ref_fleixble_size_p
to return false for it, I think. 
We might add a new utility routine to determine whether a ref to a struct or
union have flexible array?

[Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107952

--- Comment #13 from Qing Zhao  ---
> On Jan 25, 2023, at 2:32 AM, rguenther at suse dot de 
>  wrote:
>> 
>> A little confused here:  
>>when the structure with a trailing flexible-array member is a middle
>> field of 
>>an outer structure, GCC extension will treat it as a flexible-array
>> too? But the
>>maximum size of this flexible-array will be bounded by the size of the
>> padding
>>of that field? 
>> Is the above understanding correct?
> 
> That's correct - at least when using the get_ref_base_and_extent
> API.  I see that when using array_ref_flexible_size_p it doesn't
> return true (but it's technically not _flexible_, we just treat it with
> a bound size that doesn't match the declared bound).
For the current array_ref_flexible_size_p, we include the following 3 cases:

   A. a ref to a flexible array member at the end of a structure;
   B. a ref to an array with a different type against the original decl;
  for example:

   short a[16] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
   (*((char(*)[16])[0]))[i+8]

   C. a ref to an array that was passed as a parameter;
  for example:

   int test (uint8_t *p, uint32_t t[1][1], int n) {
   for (int i = 0; i < 4; i++, p++)
 t[i][0] = …;

It basically mean: return true if REF is to an array whose actual size might be
larger than its upper bound implies. 

I feel that the case "when the structure with a trailing flexible-array member
is a middle field of an outer structure” still fit here, but not very sure,
need more thinking...

> 
> The first is handled by the function just fine,

No, even the first case is not recognized by the current
“array_ref_flexible_size_p”, it’s not been identified as a flexible array right
now.
Shall we include this case into “array_ref_flexible_size_p”?  (It’s a GCC
extension).

> it's the second with the bound size that's not and that isn't a good fit 
> there,
> though we do handle padding to the end of a declaration where
> we return true.
> 
>> Handle them separately instead?
> 
> I'm not sure how important it is to hande the middle array
> extending to padding, ISTR there was some real world code
> "miscompiled" when treating the array domain as written.

We can leave the 2nd case in a later time to resolve -:)
>

[Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107952

--- Comment #10 from Qing Zhao  ---
> --- Comment #9 from Richard Biener  ---
> 
> GCC handles for example
> 
> struct A { char data[1]; };
> struct B { int n; struct A a; };
> 
> as if the a.data[] array is a flex-array.

Okay. Then the maximum size of __builtin_object_size for it should be -1,
right?

>  It also handles
> 
> struct C { int n; struct A a; int x; };
> 
> as if a.data[] can be up to 4 elements large (we allow an array to extend
> flexibly to padding - but only if it is trailing).

A little confused here:  
when the structure with a trailing flexible-array member is a middle
field of 
an outer structure, GCC extension will treat it as a flexible-array
too? But the
maximum size of this flexible-array will be bounded by the size of the
padding
of that field? 
Is the above understanding correct?

>  I see that's not
> consistently handled though.
> 
> I think the [] syntax should follow the C standard as what testcases are
> accepted/rejected by the frontend and any extensions there should be
> documented

Agreed, usually where these extension should be documented?

> (those are separate from what the former array_at_struct_end_p
> allowed semantically

So, your mean to leave such extension out of “array_at_struct_end_p” (the
current “array_ref_flexible_size_p”)? 
Handle them separately instead?

[Bug rtl-optimization/108132] Wrong instruction scheduling around function call with longjmp on aarch64 at O2

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108132

--- Comment #7 from Qing Zhao  ---
> On Dec 15, 2022, at 2:33 PM, pinskia at gcc dot gnu.org 
>  wrote:
> 
> 
> There is a patch in PR 57067 even which you could try to port to a newer
> compiler version and fix up.

Okay, will check that patch.
thanks.
>

[Bug target/106270] [Aarch64] -mlong-calls should be provided on aarch64 for users with large applications

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106270

--- Comment #4 from Qing Zhao  ---
> On Jul 12, 2022, at 1:02 PM, wilco at gcc dot gnu.org 
>  wrote:
> 
> Note that GCC could split huge .text sections automatically to allow insertion
> of linker veneers every 128MB.

Does GCC do this by default? Any option is needed for this functionality?

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #27 from Qing Zhao  ---
> On Jun 14, 2022, at 11:39 AM, siddhesh at gcc dot gnu.org 
>  wrote:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836
> 
> --- Comment #26 from Siddhesh Poyarekar  ---
> (In reply to qinzhao from comment #25)
>> So, based on all the discussion so far, how about the following:
>> 
>> ** add the following gcc option:
>> 
>> -fstrict-flex-arrays=[0|1|2|3]
>> 
>> when -fstrict-flex-arrays=0:
>> treat all trailing arrays as flexible arrays. the default behavior;
> 
> Wouldn't this be -fno-strict-flex-arrays, i.e. the current behaviour?

Yes, it’s the same.  =0 is aliased with -fno-strict-flex-arrays.

The point is, the larger the value of LEVEL, the stricter with treating the
flexing array.

i.e, 0 is the least strict, and 3 is the strictest mode.

But we can delete the level 0 if not necessary.
> 
>> when -fstrict-flex-arrays=1:
>> Only treating [], [0], and [1] as flexible array;
>> 
>> when -fstrict-flex-arrays=2:
>> Only treating [] and [0] as flexible array;
>> 
>> when -fstrict-flex-arrays=3:
>> Only treating [] as flexible array; The strictest level.
> 
> If yes, then you end up having:
> 
> -fstrict-flex-arrays=[1|2|3]
> 
> with, I suppose, 1 as the default based on Jakub's comment about maximum
> compatibility support.
Yes.  And 3 is the one Kees requested for kernel usage.

[Bug target/101891] Adjust -fzero-call-used-regs to always use XOR

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101891

--- Comment #8 from Qing Zhao  ---
> On May 24, 2022, at 11:42 AM, arjan at linux dot intel.com 
>  wrote:
> 
> --- Comment #7 from Arjan van de Ven  ---
> from a performance angle, the xor-only sequence is not so great at all; modern
> CPUs are really good at eliminating mov's so that's why the code originally 
> was
> added to do a combo of xor and mov..

Are you saying that the Xor-only sequence is slower than the previous XOR + MOV
sequence?
If so, can you explain a little bit more on why? And do you have any data for
this claim?

Thanks.

[Bug middle-end/105539] -ftrivial-auto-var-init=zero happening too late?

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105539

--- Comment #9 from Qing Zhao  ---
> It's a conditional uninit use which we do not warn on early by design
> (for the fear of too many false positives, that is).
Okay. 
> 
>> 2. the ccp optimization deletes the if (z) statement completely:
>> 
>> [opc@qinzhao-aarch64-ol8 105539]$ cat t.c.034t.ccp1
>> 
>> ;; Function x (x, funcdef_no=0, decl_uid=3591, cgraph_uid=1, symbol_order=0)
>> 
>> Removing basic block 3
>> Merging blocks 2 and 4
>> int x (int z)
>> {
>>  int y;
>> 
>>   :
>>  return 10;
>> 
>> }
> 
> That's expected from optimistic lattice propagation which merges TOP with
> the constant.

If y is initialized to 0 at declaration site, the “if z” statement is NOT
deleted by CCP. 

However, per design of -ftrivial-auto-var-init, we intent to not treat

y = .DEFERRED_INIT (4, 2, &"y"[0]);
Same as
y= 0 

In order to keep -Wuninitialized warning analysis working as expected.

So, yes, I agree that this issue is unavoided based on the current design. 

> 
>> are the above two bugs?
> 
> Well, it works as designed.  But sure, that we fail to diagnose the uninit
> use is unfortunate and there exist bugs for this specific issue already.

[Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #20 from Qing Zhao  ---
> On Feb 21, 2022, at 1:56 AM, rguenth at gcc dot gnu.org 
>  wrote:
>> 
>> my question is, is the "info" in __builtin_clear_padding() a REAL use
>> of "info"? is it correct to report the uninitialized use message for it?
> 
> As said I think reads emitted from __builtin_clear_padding lowering should
> not be considered for (uninit) diagnostics.
Okay.

[Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #14 from Qing Zhao  ---
> On Feb 17, 2022, at 1:54 AM, rguenther at suse dot de 
>  wrote:
>> 
>> If padding clearing is exposed too late till RTL expansion, some tree
>> optimization will not be able to be applied on the expanded stores? 
> 
> Doesn't the same argument apply to .DEFERRED_INIT itself?  Dependent
> on the .DEFERRED_INIT expansion strathegy the padding clearing might
> be unneccessary (for example when using memset())?

Yes, because we use memset to initialize auto-var when it’s in memory, we do
not insert a call to
__builtin_clear_padding to zero initialization.  We only insert
__builtin_clear_padding to pattern
Initialization to set the padding to zeroes. That’s why this bug only exposed
with pattern init.
> 
>> the approach to mark the load "MEM" as not needing a warning might be better?
> 
> It's probably a good thing anyway, the 'R' in the RMW cycle isn't really
> a use.
If it’s not a real use, should we exclude this case from emitting the
uninitialized warning directly?

[Bug middle-end/104550] bogus warning from -Wuninitialized + -ftrivial-auto-var-init=pattern

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104550

--- Comment #5 from Qing Zhao  ---
> On Feb 15, 2022, at 3:38 PM, pinskia at gcc dot gnu.org 
>  wrote:
> Maybe __builtin_clear_padding lowering should mark the load "MEM[(struct
> vx_audio_level *)]" as not needing a warning.
> 
This sound like a good idea, I will study a little bit more on this direction.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586

--- Comment #27 from Qing Zhao  ---
> On Feb 11, 2022, at 11:47 AM, jakub at gcc dot gnu.org 
>  wrote:
> But if I do:
> struct C { char a; int b; char c; long d; C () : a (42), b (42), c (42), d 
> (42)
> {} };
> void bar (C *);
> void
> foo ()
> {
>  C c;
>  bar ();
> }
> then *.gimple is:
>  c = .DEFERRED_INIT (24, 1, &"c"[0]);
>  __builtin_clear_padding (, 0B, 1);
>  C::C ();
>  bar ();
> ...
> void C::C (struct C * const this)
> {
>  *this = {CLOBBER};
>  {
>this->a = 42;
>this->b = 42;
>this->c = 42;
>this->d = 42;
>  }
> }
> After einline this is:
>  c = .DEFERRED_INIT (24, 1, &"c"[0]);
>  MEM  [(struct C *) + 1B] = {};
>  MEM  [(struct C *) + 9B] = {};
>  c ={v} {CLOBBER};
>  c.a = 42;
>  c.b = 42;
>  c.c = 42;
>  c.d = 42;
>  bar ();
> and that keeps until dse1 which optimizes that out:
>  c ={v} {CLOBBER};
>  c.a = 42;
>  c.b = 42;
>  c.c = 42;
>  c.d = 42;
>  bar ();
> so there is no zero padding initialization at all.

Does this issue only exist with -flifetime-dse=2?
When -flifetime-dse=2, the call to __builtin_clear_padding should be inserted
AFTER the
start point of the constructor of the object, otherwise it’s dead and will be
eliminated by DSE. 
And with -flifetime-dse=2, the padding initialization should be done by C++ FE
instead of middle end.
Is this understanding correct?

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586

--- Comment #23 from Qing Zhao  ---
> On Feb 11, 2022, at 9:29 AM, jason at gcc dot gnu.org 
>  wrote:
> 
> I wonder why -fauto-var-init uses builtin_clear_padding instead of just
> zero-initializing the whole object before normal initialization, as with
> value-initialization?  With a new object we don't need to get clever.

In the initial several versions of the implementation, I didn’t use
builtin_clear_padding, other that
that, I just zero-initialized the whole object before normal initialization.
However, multiple people
suggested to use builtin_clear_padding instead. Then in the later
implementations, I used
builtin_clear_padding for the padding initialization.

[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271

--- Comment #4 from Qing Zhao  ---
> 
> You should be able to simply do
> 
> ../configure --target=riscv64-linux
> make all-gcc
> 
> and use the built gcc/cc1 to debug such ICEs.
> 

Thanks. Yes, I can repeat the ICE with this gcc even though “make install”
still failed with the following error:

make[2]: Leaving directory '/home/opc/Work/GCC/crossbuild/libiberty'
/bin/sh: line 3: cd: ./c++tools: No such file or directory
make[1]: *** [Makefile:12252: install-c++tools] Error 1
make[1]: Leaving directory '/home/opc/Work/GCC/crossbuild'
make: *** [Makefile:2538: install] Error 2

Anyway, I can debug this current bug with this gcc now.

[Bug middle-end/103127] ICE in fold_convert_loc with __vector_quad and -ftrivial-auto-var-init=pattern on powerpc64*

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103127

--- Comment #9 from Qing Zhao  ---
> On Nov 8, 2021, at 1:56 PM, bergner at gcc dot gnu.org 
>  wrote:
> 
> 
> So this then?

This is better, I think.
You can send a patch review request to gcc-patch alias for more comments or
approval.

[Bug c++/102281] -ftrivial-auto-var-init=zero causes ice

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102281

--- Comment #10 from Qing Zhao  ---
> On Oct 11, 2021, at 3:29 PM, jakub at gcc dot gnu.org 
>  wrote:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102281
> 
> --- Comment #8 from Jakub Jelinek  ---
> (In reply to qinzhao from comment #7)
>> I agree that the above additional check  is necessary.
>> 
>> one question here is, can I use the routine "bool
>> clear_padding_type_may_have_padding_p (tree type)" in gimple-fold.c instead
>> of the above new function for this purpose?
> 
> Sure, you can, but just note that it is conservative and fast, it will return
> true even for types that don't have any padding.
> But for double, _Complex double, int etc. it will quickly return false.
> I guess I should use it in c-omp.c too.

Yes, the routine “clear_padding_type_may_have_padding_p” is quicker when
returning FALSE. 
When it return TRUE, might not be very accurate, at this time, we can further
call the new routine 
to make it accurate. 
Another question here, what’s the purpose for the routine
“clear_type_padding_in_mask”?

> But also note that it will return true for x86 long double/_Complex long
> double,
> and if you have a variable that isn't addressable, you need to figure out what
> to do.
Yes, we should resolve this issue too. 

>  I think it might be easiest not to clear anything,

Do you mean not to call clear padding for any variable here? Or only for
variables with type long double/_Complex long double?

> another option is to
> create a temporary, store the var value in there, clear padding and copy back,
> but in the end the padding bits will probably not stay cleared.

Why not?

> After all, e.g. on x86-64 -m64 the return value will be either in %st or
> %st/%st(1) pair and the padding isn't present there

You mean for variables with type long double/_Complex long doubles, if they are
in register (Not-addressable), they don’t have
Padding, so, don’t need to clear padding at all? (But this is only true for
x86-64)

> (but e.g. for ia32 _Complex
> long double is returned through invisible reference and padding is there,

If they are returned through invisible reference, does this mean they are
addressable?
> but
> you'd need to perform clear padding like operation during expansion).
> 
> -- 
> You are receiving this mail because:
> You are on the CC list for the bug.

[Bug target/102683] [12 Regression] ICE in set_min_and_max_values_for_integral_type, at stor-layout.c:2851

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102683

--- Comment #5 from Qing Zhao  ---
> --- Comment #4 from Richard Biener  ---
> But we should avoid
> the .DEFERRED_INIT here.  The GENERIC is
> 
>struct C y;
>  <  (void) (y = n == 1 ? TARGET_EXPR  : TARGET_EXPR  {.c=__complex__ (3.0e+0, 3.0e+0)}>) >;
> 
> so we have first
> 
>stmt 
>side-effects arg:0 
>t.i:10:5 start: t.i:10:5 finish: t.i:10:5>
> 
> and then
> 
>stmt  void>
>side-effects
>arg:0  void>
>side-effects
>arg:0  0x76558f18 void>
>side-effects
>arg:0  0x76663d20 C>
>side-effects arg:0 
>arg:1  0x76663d20 C>
> ...
> 
> so this is an INIT_EXPR which never(?) requires .DEFERRED_INIT?  Of course
> we're instrumenting the DECL_EXPR, not the INIT_EXPR and at that point
> we didn't see the INIT_EXPR.

Currently, in simplification phase, for each DECL_EXPR, we check:

  if (VAR_P (decl) && !DECL_EXTERNAL (decl))
{
  tree init = DECL_INITIAL (decl);

  If (!init. && is_var_need_auto_init (decl))
gimple_add_init_for_auto_var (decl…)


We assume that FE will put the explicit initializer of a DECL to its
DECL_INITIAL. 
Clearly for this testing case, this is not the case.

I am wondering why FE does not put the initializer of this DECL to its
DECL_INITIAL for this case?

> 
> I wonder if we want a bit on DECL_EXPR denoting whether the decl is
> fully initialized?

Yes, if FE can mark this bit, it will be easier and simpler for the
implementation.

[Bug middle-end/102285] New flag -ftrivial-auto-var-init=zero causes crash in pr82421.c

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102285

--- Comment #16 from Qing Zhao  ---
> On Oct 1, 2021, at 1:51 AM, rguenth at gcc dot gnu.org 
>  wrote:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102285
> 
> --- Comment #13 from Richard Biener  ---
> Because the variable doesn't need to be addressable.

One question:

For the following statement in the routine “fold_builtin_alloca_with_align” in
tree-ssa-ccp.c:


  /* Fold alloca to the address of the array.  */
  return fold_convert (TREE_TYPE (lhs), build_fold_addr_expr (var));

Clearly, we build a add_expr for “var”, but why not “mark_addressable” for
“var” since its address is taken?

[Bug sanitizer/102317] signed integer overflow sanitizer cannot work well with -fno-strict-overflow

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102317

--- Comment #5 from Qing Zhao  ---
> On Sep 13, 2021, at 4:45 PM, pinskia at gcc dot gnu.org 
>  wrote:
> 
>> is it possible to make -fsanitize=signed-integer-overflow work with -fwrapv?
> 
> Why would it? they conflict.

This is a feature that is requested by Kees Cook for kernel security usage. 

"the kernel builds with -fno-strict-overflow which removes
possible undefined behavior, but I still want the sanitizer to catch
this case.”

[Bug middle-end/102285] New flag -ftrivial-auto-var-init=zero causes many crashes in the testsuite

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102285

--- Comment #4 from Qing Zhao  ---
> On Sep 10, 2021, at 5:34 PM, pinskia at gcc dot gnu.org 
>  wrote:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102285
> 
> --- Comment #3 from Andrew Pinski  ---
> I wonder if most of these were fixed by r12-3447-g79f488de3036a

There are two patches by Richard Biener last night to fix two bugs:

commit 79f488de3036a4a4be08df2a782e6eb02419db19
Author: Richard Biener 
Date:   Fri Sep 10 12:28:09 2021 +0200

middle-end/102273 - avoid ICE with auto-init and nested functions

This refactors expansion to consider non-decl LHS.  I suspect
the is_val argument is not needed.

commit 1dae802b685937b1dc52e49d0641c75f3186ba14
Author: Richard Biener 
Date:   Fri Sep 10 10:17:24 2021 +0200

middle-end/102269 - avoid auto-init of empty types

This avoids initializing empty types for which we'll eventually
leave a .DEFERRED_INIT call without a LHS.

I checked the failed testing cases listed by David, many of them have empty
types.

So, I guess that the above two patches might already fix these failures.


> 
> -- 
> You are receiving this mail because:
> You are on the CC list for the bug.

[Bug middle-end/101586] ICE:in clear_padding_type, at gimple-fold.c:4783 with call to __builtin_clear_padding for C++

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586

--- Comment #9 from Qing Zhao  ---
> On Jul 23, 2021, at 10:34 AM, jakub at gcc dot gnu.org 
>  wrote:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586
> 
> --- Comment #8 from Jakub Jelinek  ---
> (In reply to Qing Zhao from comment #7)
>> So, is such type information generated by C++FE correct?
> 
> Yes.  It is needed that way to follow the Itanium C++ ABI.

Okay. I see.
Then with such type info, can clear_type_padding fill all the paddings for such
type?
> 
>>> BTW, if you are using the clear padding code for -ftrivial*, unless it is
>>> clear_type_padding_in_mask, it can error e.g. on flexible array members, 
>>> which
>>> is fine for the builtin, but probably not fine for -ftrivial*.
>> 
>> I noticed this,  and then I fixed this by adding a third argument for
>> __builtin_clear_padding to indicate whether it’s for auto init or not.
>> If for auto init, then not emit the error, is this fix good?
>> 
>> (BTW, what’s clear_type_padding_in_mask try to do? Should I use it instead?)
> 
> I'd need to see a patch.  

Do you want me send my current patch to you for some comments for this part?
Then I can email to you with a separate email.
> The builtin itself has a single argument and
> shouldn't accept more than one, gimple_fold_builtin_clear_padding has also 
> just
> a single argument.

Currently, in gimplification phase, we already added one more argument to this
call: (gimplify.c)


  case BUILT_IN_CLEAR_PADDING:
if (call_expr_nargs (*expr_p) == 1)
  {
/* Remember the original type of the argument in an internal
   dummy second argument, as in GIMPLE pointer conversions are
   useless.  */
p = CALL_EXPR_ARG (*expr_p, 0);
*expr_p
  = build_call_expr_loc (EXPR_LOCATION (*expr_p), fndecl, 2, p,
 build_zero_cst (TREE_TYPE (p)));
return GS_OK;
=

So, I just added one more argument in the above to distinguish the auto init.
>  clear_type_padding_in_mask doesn't emit code to clear any
> memory, but fills in an array with bitmask which bits contain padding and 
> which
> don't.
> 
> -- 
> You are receiving this mail because:
> You reported the bug.

[Bug middle-end/101586] ICE:in clear_padding_type, at gimple-fold.c:4783 with call to __builtin_clear_padding for C++

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586

--- Comment #7 from Qing Zhao  ---
> On Jul 23, 2021, at 10:10 AM, jakub at gcc dot gnu.org 
>  wrote:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101586
> 
> --- Comment #6 from Jakub Jelinek  ---
> It is related to the weird FIELD_DECLs the C++ FE creates for the virtual
> inheritence, there is e.g. a FIELD_DECL with B type (where B has 16 byte size
> and includes the 8 byte virtual pointer and 1 byte A), but the size of the
> FIELD_DECL is just 8 bytes, which is something the clear padding code didn't
> expect to see.

So, is such type information generated by C++FE correct?
> 
> BTW, if you are using the clear padding code for -ftrivial*, unless it is
> clear_type_padding_in_mask, it can error e.g. on flexible array members, which
> is fine for the builtin, but probably not fine for -ftrivial*.

I noticed this,  and then I fixed this by adding a third argument for
__builtin_clear_padding to indicate whether it’s for auto init or not.
If for auto init, then not emit the error, is this fix good?

(BTW, what’s clear_type_padding_in_mask try to do? Should I use it instead?)

[Bug rtl-optimization/99421] ICE:in code_motion_process_successors, at sel-sched.c:6389 on aarch64

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99421

--- Comment #8 from Qing Zhao  ---
> On Mar 8, 2021, at 11:58 AM, marxin at gcc dot gnu.org 
>  wrote:
> 
> Sure. I used C-Vise:
> https://github.com/marxin/cvise

>From my understanding, cvise is similar as creduce, but faster.
So, I am still wondering why creduce cannot reduce testing cases when using
profiling feedback due to
Profiling mismatch? But cvise can do this? Does Cvise reduce source code and
.gcda file at the same 
time?

[Bug testsuite/97680] [11 Regression] new test case c-c++-common/zero-scratch-regs-10.c in r11-4578 has excess errors

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97680

--- Comment #10 from Qing Zhao  ---
> but it will still fail on all targets but x86_64 (and now powerpc).  Qinzhao,
> what's the list of targets this is supported?

I believe that the targets that currently support this feature are:
x86-64
aarch64
sparc

The original patch supported x86-64 and aarch64, later the following patch
https://gcc.gnu.org/pipermail/gcc-cvs/2020-December/338342.html


Support sparc

[Bug target/97715] [11 Regression] ICE in insn_default_length, at config/i386/i386.md:15325 since r11-4578-gd10f3e900b0377b4

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97715

--- Comment #21 from Qing Zhao  ---
> --- Comment #19 from Jakub Jelinek  ---
> And, actually the function.c change is probably unnecessary, because the
>  if (!crtl->abi->clobbers_full_reg_p (regno))
>continue;
>  if (!fixed_regs[i])
>continue;
> should already rule them out.
>  operand_reg_set &= accessible_reg_set;
> ...
>  /* If a register is too limited to be treated as a register operand,
> then it should never be allocated to a pseudo.  */
>  if (!TEST_HARD_REG_BIT (operand_reg_set, i))
>fixed_regs[i] = 1;

So, fixed_regs should already include the information of accessible_reg_set, no
need to check accessible_reg_set anymore.

[Bug target/97715] [11 Regression] ICE in insn_default_length, at config/i386/i386.md:15325 since r11-4578-gd10f3e900b0377b4

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97715

--- Comment #20 from Qing Zhao  ---
> On Nov 4, 2020, at 11:48 AM, jakub at gcc dot gnu.org 
>  wrote:
> 
> --- Comment #18 from Jakub Jelinek  ---
> --- gcc/function.c.jj   2020-10-31 17:41:19.756740009 +0100
> +++ gcc/function.c  2020-11-04 17:56:53.790403571 +0100
> @@ -5871,6 +5871,8 @@ gen_call_used_regs_seq (rtx_insn *ret, u
>continue;
>   if (fixed_regs[regno])
>continue;
> +  if (!TEST_HARD_REG_BIT (accessible_reg_set, regno))
> +   continue;
>   if (REGNO_REG_SET_P (live_out, regno))
>continue;
>   if (only_gpr
> --- gcc/testsuite/gcc.target/i386/zero-scratch-regs-32.c.jj 2020-11-04
> 17:59:19.063795531 +0100
> +++ gcc/testsuite/gcc.target/i386/zero-scratch-regs-32.c2020-11-04
> 17:59:01.956984879 +0100
> @@ -0,0 +1,5 @@
> +/* PR target/97715 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fzero-call-used-regs=all -mno-80387" } */
> +
> +#include "../../c-c++-common/zero-scratch-regs-10.c"
> 
> isn't enough though, because while the st to st(7) registers are inaccessible
> with -mno-80387, the MMX registers are accessible, so I think the
> zero_all_st_registers change is needed too.

Yeh, I noticed the same issue too.
I will add the fix you proposed previously.

[Bug target/97715] [11 Regression] ICE in insn_default_length, at config/i386/i386.md:15325 since r11-4578-gd10f3e900b0377b4

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97715

--- Comment #17 from Qing Zhao  ---
> On Nov 4, 2020, at 10:08 AM, ubizjak at gmail dot com 
>  wrote:
>> 
>> I used the following in middle end to exclude fixed registers from being
>> zeroed:
>>  if (fixed_regs[regno])
>>continue;
>> So, looks like that ST registers are not included in fixed_regs when
>> !TARGET_80387. 
>> Is this a bug in gcc?
> 
> fixed_regs members are only set with -ffixed-REG, in addition to members,
> initialized by the target-dependent initializer. It is not a bug.
Yes.

Then, we need to check whether the register is in “accessible_reg_set” to
exclude it from being
Zeroed as Jacub suggested in  Comment #14.  That looks like a better fix.

[Bug target/97715] [11 Regression] ICE in insn_default_length, at config/i386/i386.md:15325 since r11-4578-gd10f3e900b0377b4

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97715

--- Comment #13 from Qing Zhao  ---
> On Nov 4, 2020, at 10:04 AM, jakub at gcc dot gnu.org 
>  wrote:
> --- Comment #11 from Jakub Jelinek  ---
> I think you should do:
> --- gcc/function.c  2020-10-31 17:41:19.756740009 +0100
> +++ gcc/function.c  2020-11-04 17:02:51.199298173 +0100
> @@ -5871,6 +5871,8 @@ gen_call_used_regs_seq (rtx_insn *ret, u
>continue;
>   if (fixed_regs[regno])
>continue;
> +  if (!TEST_HARD_REG_BIT (accessible_reg_set, regno))
> +   continue;
>   if (REGNO_REG_SET_P (live_out, regno))\

Yes, this looks like a better solution. I will add this fix.

[Bug target/97715] [11 Regression] ICE in insn_default_length, at config/i386/i386.md:15325 since r11-4578-gd10f3e900b0377b4

[qing.zhao at oracle dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97715

--- Comment #10 from Qing Zhao  ---
> On Nov 4, 2020, at 9:45 AM, ubizjak at gmail dot com 
>  wrote:
>> fixed registers should already be excluded from zeroing. 
>> are ST registers considered FIXED registers when -mno-80387 is specified?
> 
> They used to be,

I used the following in middle end to exclude fixed registers from being
zeroed:
  if (fixed_regs[regno])
continue;
So, looks like that ST registers are not included in fixed_regs when
!TARGET_80387. 
Is this a bug in gcc?

> but now they are cleared from accessible_reg_set.
> 
>  /* If the FPU is disabled, disable the registers.  */
>  if (! (TARGET_80387 || TARGET_FLOAT_RETURNS_IN_80387))
>accessible_reg_set &= ~reg_class_contents[FLOAT_REGS];

The above means that ST registers are excluded from the accessible_reg_set,
i.e, they cannot be used in the function body anymore? Then, no need to zero
them?

Is this information available to middle end?
Or I have to delete them from the register set in i386 backend?