HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

Hi, 

I am testing the 4th patch of -ftrivial-auto-var-init with CPU2017 today, and found the following issues:

****In the dump file of “*t.i.031t.objsz1”, we have:

<bb 3> :
  __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
  __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);
  __s2_len_219 = 7;
  if (__s2_len_219 <= 3)
    goto <bb 4>; [INV]
  else
    goto <bb 9>; [INV]

  <bb 4> :
  _1 = (long unsigned int) i_175;
 

****However, after “ccp”, in “t.i.032t.ccp1”, we have:

<bb 3> :
  __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
  __s2_len_218 = .DEFERRED_INIT (7, 2);
  _36 = (long unsigned int) i_175;
  _37 = _36 * 8;
  _38 = argv_220(D) + _37;


Looks like that the optimization “ccp” replaced the first argument of the call .DEFERRED_INIT with the constant 7.
This should be avoided. 

(NOTE, this issue existed in the previous patches, however, only exposed with this version since I added more verification
code in tree-cfg.c to verify the call to .DEFERRED_INIT).

I am wondering what’s the best solution to this problem? 

Can we add any attribute to the internal function argument to prevent later optimizations that might applied on it? 
Or just update “ccp” phase to specially handle calls to .DEFERRED_INIT? (Not sure whether there are other phases have the
Same issue?)

Let me know if you have any suggestion.

Thanks a lot for your help.

Qing

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Richard Biener]

On Wed, 30 Jun 2021, Qing Zhao wrote:

> Hi, 
> 
> I am testing the 4th patch of -ftrivial-auto-var-init with CPU2017 today, and found the following issues:
> 
> ****In the dump file of “*t.i.031t.objsz1”, we have:
> 
> <bb 3> :
>   __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>   __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);

I looks like this .DEFERRED_INIT initializes an already initialized
variable.  I'd expect to only ever see default definition SSA names
as first argument to .DEFERRED_INIT.

>   __s2_len_219 = 7;
>   if (__s2_len_219 <= 3)
>     goto <bb 4>; [INV]
>   else
>     goto <bb 9>; [INV]
> 
>   <bb 4> :
>   _1 = (long unsigned int) i_175;
>  
> 
> ****However, after “ccp”, in “t.i.032t.ccp1”, we have:
> 
> <bb 3> :
>   __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>   __s2_len_218 = .DEFERRED_INIT (7, 2);
>   _36 = (long unsigned int) i_175;
>   _37 = _36 * 8;
>   _38 = argv_220(D) + _37;
> 
> 
> Looks like that the optimization “ccp” replaced the first argument of the call .DEFERRED_INIT with the constant 7.
> This should be avoided. 
> 
> (NOTE, this issue existed in the previous patches, however, only exposed with this version since I added more verification
> code in tree-cfg.c to verify the call to .DEFERRED_INIT).
> 
> I am wondering what’s the best solution to this problem? 

I think you have to trace where this "bogus" .DEFERRED_INIT comes from 
originally.  Or alternatively, if this is unavoidable, add "constant 
folding" of .DEFERRED_INIT so that defered init of an initialized
object becomes the object itself, thus retain the previous - eventually
partial - initialization only.

Richard.

> Can we add any attribute to the internal function argument to prevent later optimizations that might applied on it? 
> Or just update “ccp” phase to specially handle calls to .DEFERRED_INIT? (Not sure whether there are other phases have the
> Same issue?)
> 
> Let me know if you have any suggestion.
> 
> Thanks a lot for your help.
> 
> Qing

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

On Jun 30, 2021, at 2:46 AM, Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>> wrote:

On Wed, 30 Jun 2021, Qing Zhao wrote:

Hi,

I am testing the 4th patch of -ftrivial-auto-var-init with CPU2017 today, and found the following issues:

****In the dump file of “*t.i.031t.objsz1”, we have:

<bb 3> :
 __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
 __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);

I looks like this .DEFERRED_INIT initializes an already initialized
variable.

Yes.

For cases like the following:

int s2_len;
s2_len = 4;

i.e, the initialization is not at the declaration.

We cannot avoid initialization for such cases.

 I'd expect to only ever see default definition SSA names
as first argument to .DEFERRED_INIT.

You mean something like:
__s2_len_218 = .DEFERRED_INIT (__s2_len, 2);

?


 __s2_len_219 = 7;
 if (__s2_len_219 <= 3)
   goto <bb 4>; [INV]
 else
   goto <bb 9>; [INV]

 <bb 4> :
 _1 = (long unsigned int) i_175;


****However, after “ccp”, in “t.i.032t.ccp1”, we have:

<bb 3> :
 __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
 __s2_len_218 = .DEFERRED_INIT (7, 2);
 _36 = (long unsigned int) i_175;
 _37 = _36 * 8;
 _38 = argv_220(D) + _37;


Looks like that the optimization “ccp” replaced the first argument of the call .DEFERRED_INIT with the constant 7.
This should be avoided.

(NOTE, this issue existed in the previous patches, however, only exposed with this version since I added more verification
code in tree-cfg.c to verify the call to .DEFERRED_INIT).

I am wondering what’s the best solution to this problem?

I think you have to trace where this "bogus" .DEFERRED_INIT comes from
originally.  Or alternatively, if this is unavoidable,

This is unavoidable, I believe.

add "constant
folding" of .DEFERRED_INIT so that defered init of an initialized
object becomes the object itself, thus retain the previous - eventually
partial - initialization only.

If this additional .DEFERRED_INIT will be kept till RTL expansion phase, then it will become a real initialization:

i.e.

s2_len = 0;    //.DEFERRED_INIT expanded
s2_len = 4;    // the original initialization

Then the first initialization will be eliminated by current RTL optimization easily, right?

Qing


Richard.

Can we add any attribute to the internal function argument to prevent later optimizations that might applied on it?
Or just update “ccp” phase to specially handle calls to .DEFERRED_INIT? (Not sure whether there are other phases have the
Same issue?)

Let me know if you have any suggestion.

Thanks a lot for your help.

Qing

--
Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Richard Biener]

On Wed, 30 Jun 2021, Qing Zhao wrote:

> 
> 
> On Jun 30, 2021, at 2:46 AM, Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>> wrote:
> 
> On Wed, 30 Jun 2021, Qing Zhao wrote:
> 
> Hi,
> 
> I am testing the 4th patch of -ftrivial-auto-var-init with CPU2017 today, and found the following issues:
> 
> ****In the dump file of “*t.i.031t.objsz1”, we have:
> 
> <bb 3> :
>  __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>  __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);
> 
> I looks like this .DEFERRED_INIT initializes an already initialized
> variable.
> 
> Yes.
> 
> For cases like the following:
> 
> int s2_len;
> s2_len = 4;
> 
> i.e, the initialization is not at the declaration.
> 
> We cannot avoid initialization for such cases.

But I'd have expected

  s2_len = .DEFERRED_INIT (s2_len, 0);
  s2_len = 4;

from the above - thus the deferred init _before_ the first
"use" which is the explicit init.  How does the other order
happen to materialize?  As said, I believe it shouldn't.

>  I'd expect to only ever see default definition SSA names
> as first argument to .DEFERRED_INIT.
> 
> You mean something like:
> __s2_len_218 = .DEFERRED_INIT (__s2_len, 2);

No,

__s2_len_218 = .DEFERRED_INIT (__s2_len_217(D), 2);

> ?
> 
> 
>  __s2_len_219 = 7;
>  if (__s2_len_219 <= 3)
>    goto <bb 4>; [INV]
>  else
>    goto <bb 9>; [INV]
> 
>  <bb 4> :
>  _1 = (long unsigned int) i_175;
> 
> 
> ****However, after “ccp”, in “t.i.032t.ccp1”, we have:
> 
> <bb 3> :
>  __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>  __s2_len_218 = .DEFERRED_INIT (7, 2);
>  _36 = (long unsigned int) i_175;
>  _37 = _36 * 8;
>  _38 = argv_220(D) + _37;
> 
> 
> Looks like that the optimization “ccp” replaced the first argument of the call .DEFERRED_INIT with the constant 7.
> This should be avoided.
> 
> (NOTE, this issue existed in the previous patches, however, only exposed with this version since I added more verification
> code in tree-cfg.c to verify the call to .DEFERRED_INIT).
> 
> I am wondering what’s the best solution to this problem?
> 
> I think you have to trace where this "bogus" .DEFERRED_INIT comes from
> originally.  Or alternatively, if this is unavoidable,
> 
> This is unavoidable, I believe.

I see but don't believe it yet ;)

> add "constant
> folding" of .DEFERRED_INIT so that defered init of an initialized
> object becomes the object itself, thus retain the previous - eventually
> partial - initialization only.
> 
> If this additional .DEFERRED_INIT will be kept till RTL expansion phase, then it will become a real initialization:
> 
> i.e.
> 
> s2_len = 0;    //.DEFERRED_INIT expanded
> s2_len = 4;    // the original initialization
> 
> Then the first initialization will be eliminated by current RTL optimization easily, right?

Well, in your example above it's effectively elimiated by GIMPLE 
optimization.  IIRC you're using the first argument of .DEFERRED_INIT
for diagnostic purposes only, correct?

Richard.

> Qing
> 
> 
> Richard.
> 
> Can we add any attribute to the internal function argument to prevent later optimizations that might applied on it?
> Or just update “ccp” phase to specially handle calls to .DEFERRED_INIT? (Not sure whether there are other phases have the
> Same issue?)
> 
> Let me know if you have any suggestion.
> 
> Thanks a lot for your help.
> 
> Qing
> 
> --
> Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

> On Jun 30, 2021, at 9:39 AM, Richard Biener <rguenther@suse.de> wrote:
> 
> On Wed, 30 Jun 2021, Qing Zhao wrote:
> 
>> 
>> 
>> On Jun 30, 2021, at 2:46 AM, Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>> wrote:
>> 
>> On Wed, 30 Jun 2021, Qing Zhao wrote:
>> 
>> Hi,
>> 
>> I am testing the 4th patch of -ftrivial-auto-var-init with CPU2017 today, and found the following issues:
>> 
>> ****In the dump file of “*t.i.031t.objsz1”, we have:
>> 
>> <bb 3> :
>> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>> __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);
>> 
>> I looks like this .DEFERRED_INIT initializes an already initialized
>> variable.
>> 
>> Yes.
>> 
>> For cases like the following:
>> 
>> int s2_len;
>> s2_len = 4;
>> 
>> i.e, the initialization is not at the declaration.
>> 
>> We cannot avoid initialization for such cases.
> 
> But I'd have expected
> 
>  s2_len = .DEFERRED_INIT (s2_len, 0);
>  s2_len = 4;
> 
> from the above - thus the deferred init _before_ the first
> "use" which is the explicit init.  How does the other order
> happen to materialize?  As said, I believe it shouldn't.

Right, this is strange to me too. … don’t quite understand. 
Maybe I need to debug deeper into “ccp” phase to understand this behavior.

> 
>> I'd expect to only ever see default definition SSA names
>> as first argument to .DEFERRED_INIT.
>> 
>> You mean something like:
>> __s2_len_218 = .DEFERRED_INIT (__s2_len, 2);
> 
> No,
> 
> __s2_len_218 = .DEFERRED_INIT (__s2_len_217(D), 2);

Okay, I see.

Then please see the following:

******In “GIMPLE” dump:

    try
      {
        C = .DEFERRED_INIT (C, 2);
        syshandle = .DEFERRED_INIT (syshandle, 2);
        ba = .DEFERRED_INIT (ba, 2);
        {
          int i;

          i = .DEFERRED_INIT (i, 2);
          i = 0;
          goto <D.18219>;
          <D.18218>:
          {
            size_t __s1_len;
            size_t __s2_len;

            __s1_len = .DEFERRED_INIT (__s1_len, 2);
            __s2_len = .DEFERRED_INIT (__s2_len, 2);
            __s2_len = 7;
            if (__s2_len <= 3) goto <D.18230>; else goto <D.18231>;

NOTE, in the above, in addition to “__s2_len”, “I” also has an original initialization already. 

******Then in “ssa” dump:

  <bb 2> :
  C_200 = .DEFERRED_INIT (C_199(D), 2);
  syshandle = .DEFERRED_INIT (syshandle, 2);
  ba_204 = .DEFERRED_INIT (ba_203(D), 2);
  i_206 = .DEFERRED_INIT (i_205(D), 2);
  i_207 = 0;
  goto <bb 37>; [INV]

  <bb 3> :
  __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
  __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);
  __s2_len_219 = 7;
  if (__s2_len_219 <= 3)

NOTE, in the above, “I” has:

i_206 = .DEFERRED_INIT (i_205(D), 2);

But, for “__s2_len”, it is:

__s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);

Not sure why such difference?
> 
>> ?
>> 
>> 
>> __s2_len_219 = 7;
>> if (__s2_len_219 <= 3)
>>   goto <bb 4>; [INV]
>> else
>>   goto <bb 9>; [INV]
>> 
>> <bb 4> :
>> _1 = (long unsigned int) i_175;
>> 
>> 
>> ****However, after “ccp”, in “t.i.032t.ccp1”, we have:
>> 
>> <bb 3> :
>> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>> __s2_len_218 = .DEFERRED_INIT (7, 2);
>> _36 = (long unsigned int) i_175;
>> _37 = _36 * 8;
>> _38 = argv_220(D) + _37;
>> 
>> 
>> Looks like that the optimization “ccp” replaced the first argument of the call .DEFERRED_INIT with the constant 7.
>> This should be avoided.
>> 
>> (NOTE, this issue existed in the previous patches, however, only exposed with this version since I added more verification
>> code in tree-cfg.c to verify the call to .DEFERRED_INIT).
>> 
>> I am wondering what’s the best solution to this problem?
>> 
>> I think you have to trace where this "bogus" .DEFERRED_INIT comes from
>> originally.  Or alternatively, if this is unavoidable,
>> 
>> This is unavoidable, I believe.
> 
> I see but don't believe it yet ;)

In order to add initialization for all possible uninitialized auto variables, we check whether the auto variable is initialized at the
Declaration, we don’t follow the data flow to check the later possible initialization. Therefore, we might add more initialization than 
Necessary.  However, most of such additional initialization should be eliminated easily by later optimizations.  Especially after 
the call to .DEFERRED_INIT being expanded to real initialization. 
> 
>> add "constant
>> folding" of .DEFERRED_INIT so that defered init of an initialized
>> object becomes the object itself, thus retain the previous - eventually
>> partial - initialization only.
>> 
>> If this additional .DEFERRED_INIT will be kept till RTL expansion phase, then it will become a real initialization:
>> 
>> i.e.
>> 
>> s2_len = 0;    //.DEFERRED_INIT expanded
>> s2_len = 4;    // the original initialization
>> 
>> Then the first initialization will be eliminated by current RTL optimization easily, right?
> 
> Well, in your example above it's effectively elimiated by GIMPLE 
> optimization.  IIRC you're using the first argument of .DEFERRED_INIT
> for diagnostic purposes only, correct?

In Previous patches, Yes, the first one only for diagnostic purpose, so we can just delete the first argument and make the call as

temp = .DEFERRED_INIT (init_type);

Then this should resolve this current issue?

But in this patch,  in order to merge VLA and NON_VLA, the “size” info of the VLA variable will be carried by the first argument as following:

temp = .DEFERRED_INIT (SIZE_EXPR (temp), init_type).

So, we cannot delete the first argument anymore. 

Can I change the parameter of .DEFERRED_INIT a little bit as following:

Temp = .DEFERRED_INIT ( NULL or SIZE_EXPR, init_type)

The first parameter might be NULL for non-vla, and a SIZE_EXPR for vla? 

thanks.

Qing




> 
> Richard.
> 
>> Qing
>> 
>> 
>> Richard.
>> 
>> Can we add any attribute to the internal function argument to prevent later optimizations that might applied on it?
>> Or just update “ccp” phase to specially handle calls to .DEFERRED_INIT? (Not sure whether there are other phases have the
>> Same issue?)
>> 
>> Let me know if you have any suggestion.
>> 
>> Thanks a lot for your help.
>> 
>> Qing
>> 
>> --
>> Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>> 
>> 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

I came up with a very simple testing case that can repeat the same issue:

[qinzhao@localhost gcc]$ cat t.c
extern void bar (int);
void foo (int a)
{
  int i;
  for (i = 0; i < a; i++) {
    if (__extension__({int size2; 
	size2 = 4;
	size2 > 5;}))
    bar (a);
  }
}

[qinzhao@localhost gcc]$ /home/qinzhao/Work/GCC/gcc_build_debug/gcc/xgcc -B/home/qinzhao/Work/GCC/gcc_build_debug/gcc/ -std=c99   -m64  -march=native -ftrivial-auto-var-init=zero t.c -fdump-tree-all  -O1
t.c: In function ‘foo’:
t.c:11:1: error: ‘DEFFERED_INIT’ calls should have the same LHS as the first argument
   11 | }
      | ^
size2_12 = .DEFERRED_INIT (4, 2);
during GIMPLE pass: ccp
dump file: a-t.c.032t.ccp1
t.c:11:1: internal compiler error: verify_gimple failed
0x143ee47 verify_gimple_in_cfg(function*, bool)
	../../latest_gcc/gcc/tree-cfg.c:5501
0x122d799 execute_function_todo
	../../latest_gcc/gcc/passes.c:2042
0x122c74b do_per_function
	../../latest_gcc/gcc/passes.c:1687
0x122d986 execute_todo
	../../latest_gcc/gcc/passes.c:2096
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

In this testing case, both “I” and “size2” are auto vars that are not initialized at declaration but are initialized later by assignment.
However, “I” doesn’t have any issue, but “size2” has such issue. 

******“ssa” dump:

  <bb 2> :
  i_7 = .DEFERRED_INIT (i_6(D), 2);
  i_8 = 0;
  goto <bb 6>; [INV]

  <bb 3> :
  size2_12 = .DEFERRED_INIT (size2_3, 2);
  size2_13 = 4;

******”ccp1” dump:

  <bb 2> :
  i_7 = .DEFERRED_INIT (i_6(D), 2);
  goto <bb 4>; [INV]

  <bb 3> :
  size2_12 = .DEFERRED_INIT (4, 2);

So, I am wondering:  Is it possible that “ssa” phase have a bug ?

Qing

> On Jun 30, 2021, at 9:39 AM, Richard Biener <rguenther@suse.de> wrote:
> 
> On Wed, 30 Jun 2021, Qing Zhao wrote:
> 
>> 
>> 
>> On Jun 30, 2021, at 2:46 AM, Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>> wrote:
>> 
>> On Wed, 30 Jun 2021, Qing Zhao wrote:
>> 
>> Hi,
>> 
>> I am testing the 4th patch of -ftrivial-auto-var-init with CPU2017 today, and found the following issues:
>> 
>> ****In the dump file of “*t.i.031t.objsz1”, we have:
>> 
>> <bb 3> :
>> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>> __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);
>> 
>> I looks like this .DEFERRED_INIT initializes an already initialized
>> variable.
>> 
>> Yes.
>> 
>> For cases like the following:
>> 
>> int s2_len;
>> s2_len = 4;
>> 
>> i.e, the initialization is not at the declaration.
>> 
>> We cannot avoid initialization for such cases.
> 
> But I'd have expected
> 
>  s2_len = .DEFERRED_INIT (s2_len, 0);
>  s2_len = 4;
> 
> from the above - thus the deferred init _before_ the first
> "use" which is the explicit init.  How does the other order
> happen to materialize?  As said, I believe it shouldn't.
> 
>> I'd expect to only ever see default definition SSA names
>> as first argument to .DEFERRED_INIT.
>> 
>> You mean something like:
>> __s2_len_218 = .DEFERRED_INIT (__s2_len, 2);
> 
> No,
> 
> __s2_len_218 = .DEFERRED_INIT (__s2_len_217(D), 2);
> 
>> ?
>> 
>> 
>> __s2_len_219 = 7;
>> if (__s2_len_219 <= 3)
>>   goto <bb 4>; [INV]
>> else
>>   goto <bb 9>; [INV]
>> 
>> <bb 4> :
>> _1 = (long unsigned int) i_175;
>> 
>> 
>> ****However, after “ccp”, in “t.i.032t.ccp1”, we have:
>> 
>> <bb 3> :
>> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>> __s2_len_218 = .DEFERRED_INIT (7, 2);
>> _36 = (long unsigned int) i_175;
>> _37 = _36 * 8;
>> _38 = argv_220(D) + _37;
>> 
>> 
>> Looks like that the optimization “ccp” replaced the first argument of the call .DEFERRED_INIT with the constant 7.
>> This should be avoided.
>> 
>> (NOTE, this issue existed in the previous patches, however, only exposed with this version since I added more verification
>> code in tree-cfg.c to verify the call to .DEFERRED_INIT).
>> 
>> I am wondering what’s the best solution to this problem?
>> 
>> I think you have to trace where this "bogus" .DEFERRED_INIT comes from
>> originally.  Or alternatively, if this is unavoidable,
>> 
>> This is unavoidable, I believe.
> 
> I see but don't believe it yet ;)
> 
>> add "constant
>> folding" of .DEFERRED_INIT so that defered init of an initialized
>> object becomes the object itself, thus retain the previous - eventually
>> partial - initialization only.
>> 
>> If this additional .DEFERRED_INIT will be kept till RTL expansion phase, then it will become a real initialization:
>> 
>> i.e.
>> 
>> s2_len = 0;    //.DEFERRED_INIT expanded
>> s2_len = 4;    // the original initialization
>> 
>> Then the first initialization will be eliminated by current RTL optimization easily, right?
> 
> Well, in your example above it's effectively elimiated by GIMPLE 
> optimization.  IIRC you're using the first argument of .DEFERRED_INIT
> for diagnostic purposes only, correct?
> 
> Richard.
> 
>> Qing
>> 
>> 
>> Richard.
>> 
>> Can we add any attribute to the internal function argument to prevent later optimizations that might applied on it?
>> Or just update “ccp” phase to specially handle calls to .DEFERRED_INIT? (Not sure whether there are other phases have the
>> Same issue?)
>> 
>> Let me know if you have any suggestion.
>> 
>> Thanks a lot for your help.
>> 
>> Qing
>> 
>> --
>> Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>> 
>> 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Andrew Pinski]

On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> I came up with a very simple testing case that can repeat the same issue:
>
> [qinzhao@localhost gcc]$ cat t.c
> extern void bar (int);
> void foo (int a)
> {
>   int i;
>   for (i = 0; i < a; i++) {
>     if (__extension__({int size2;
>         size2 = 4;
>         size2 > 5;}))
>     bar (a);
>   }
> }

You should show the full dump,
What we have is the following:



size2_3 = PHI <size2_1(D), size2_13>
   <bb 3> :

size2_12 = .DEFERRED_INIT (size2_3, 2);
size2_13 = 4;

So CCP decides to propagate 4 into the PHI and then decides size2_1(D)
is undefined so size2_3 is then considered 4 and propagates it into
the .DEFERRED_INIT.

Thanks,
Andrew


>
> [qinzhao@localhost gcc]$ /home/qinzhao/Work/GCC/gcc_build_debug/gcc/xgcc -B/home/qinzhao/Work/GCC/gcc_build_debug/gcc/ -std=c99   -m64  -march=native -ftrivial-auto-var-init=zero t.c -fdump-tree-all  -O1
> t.c: In function ‘foo’:
> t.c:11:1: error: ‘DEFFERED_INIT’ calls should have the same LHS as the first argument
>    11 | }
>       | ^
> size2_12 = .DEFERRED_INIT (4, 2);
> during GIMPLE pass: ccp
> dump file: a-t.c.032t.ccp1
> t.c:11:1: internal compiler error: verify_gimple failed
> 0x143ee47 verify_gimple_in_cfg(function*, bool)
>         ../../latest_gcc/gcc/tree-cfg.c:5501
> 0x122d799 execute_function_todo
>         ../../latest_gcc/gcc/passes.c:2042
> 0x122c74b do_per_function
>         ../../latest_gcc/gcc/passes.c:1687
> 0x122d986 execute_todo
>         ../../latest_gcc/gcc/passes.c:2096
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <https://gcc.gnu.org/bugs/> for instructions.
>
> In this testing case, both “I” and “size2” are auto vars that are not initialized at declaration but are initialized later by assignment.
> However, “I” doesn’t have any issue, but “size2” has such issue.
>
> ******“ssa” dump:
>
>   <bb 2> :
>   i_7 = .DEFERRED_INIT (i_6(D), 2);
>   i_8 = 0;
>   goto <bb 6>; [INV]
>
>   <bb 3> :
>   size2_12 = .DEFERRED_INIT (size2_3, 2);
>   size2_13 = 4;
>
> ******”ccp1” dump:
>
>   <bb 2> :
>   i_7 = .DEFERRED_INIT (i_6(D), 2);
>   goto <bb 4>; [INV]
>
>   <bb 3> :
>   size2_12 = .DEFERRED_INIT (4, 2);
>
> So, I am wondering:  Is it possible that “ssa” phase have a bug ?
>
> Qing
>
> > On Jun 30, 2021, at 9:39 AM, Richard Biener <rguenther@suse.de> wrote:
> >
> > On Wed, 30 Jun 2021, Qing Zhao wrote:
> >
> >>
> >>
> >> On Jun 30, 2021, at 2:46 AM, Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>> wrote:
> >>
> >> On Wed, 30 Jun 2021, Qing Zhao wrote:
> >>
> >> Hi,
> >>
> >> I am testing the 4th patch of -ftrivial-auto-var-init with CPU2017 today, and found the following issues:
> >>
> >> ****In the dump file of “*t.i.031t.objsz1”, we have:
> >>
> >> <bb 3> :
> >> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
> >> __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);
> >>
> >> I looks like this .DEFERRED_INIT initializes an already initialized
> >> variable.
> >>
> >> Yes.
> >>
> >> For cases like the following:
> >>
> >> int s2_len;
> >> s2_len = 4;
> >>
> >> i.e, the initialization is not at the declaration.
> >>
> >> We cannot avoid initialization for such cases.
> >
> > But I'd have expected
> >
> >  s2_len = .DEFERRED_INIT (s2_len, 0);
> >  s2_len = 4;
> >
> > from the above - thus the deferred init _before_ the first
> > "use" which is the explicit init.  How does the other order
> > happen to materialize?  As said, I believe it shouldn't.
> >
> >> I'd expect to only ever see default definition SSA names
> >> as first argument to .DEFERRED_INIT.
> >>
> >> You mean something like:
> >> __s2_len_218 = .DEFERRED_INIT (__s2_len, 2);
> >
> > No,
> >
> > __s2_len_218 = .DEFERRED_INIT (__s2_len_217(D), 2);
> >
> >> ?
> >>
> >>
> >> __s2_len_219 = 7;
> >> if (__s2_len_219 <= 3)
> >>   goto <bb 4>; [INV]
> >> else
> >>   goto <bb 9>; [INV]
> >>
> >> <bb 4> :
> >> _1 = (long unsigned int) i_175;
> >>
> >>
> >> ****However, after “ccp”, in “t.i.032t.ccp1”, we have:
> >>
> >> <bb 3> :
> >> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
> >> __s2_len_218 = .DEFERRED_INIT (7, 2);
> >> _36 = (long unsigned int) i_175;
> >> _37 = _36 * 8;
> >> _38 = argv_220(D) + _37;
> >>
> >>
> >> Looks like that the optimization “ccp” replaced the first argument of the call .DEFERRED_INIT with the constant 7.
> >> This should be avoided.
> >>
> >> (NOTE, this issue existed in the previous patches, however, only exposed with this version since I added more verification
> >> code in tree-cfg.c to verify the call to .DEFERRED_INIT).
> >>
> >> I am wondering what’s the best solution to this problem?
> >>
> >> I think you have to trace where this "bogus" .DEFERRED_INIT comes from
> >> originally.  Or alternatively, if this is unavoidable,
> >>
> >> This is unavoidable, I believe.
> >
> > I see but don't believe it yet ;)
> >
> >> add "constant
> >> folding" of .DEFERRED_INIT so that defered init of an initialized
> >> object becomes the object itself, thus retain the previous - eventually
> >> partial - initialization only.
> >>
> >> If this additional .DEFERRED_INIT will be kept till RTL expansion phase, then it will become a real initialization:
> >>
> >> i.e.
> >>
> >> s2_len = 0;    //.DEFERRED_INIT expanded
> >> s2_len = 4;    // the original initialization
> >>
> >> Then the first initialization will be eliminated by current RTL optimization easily, right?
> >
> > Well, in your example above it's effectively elimiated by GIMPLE
> > optimization.  IIRC you're using the first argument of .DEFERRED_INIT
> > for diagnostic purposes only, correct?
> >
> > Richard.
> >
> >> Qing
> >>
> >>
> >> Richard.
> >>
> >> Can we add any attribute to the internal function argument to prevent later optimizations that might applied on it?
> >> Or just update “ccp” phase to specially handle calls to .DEFERRED_INIT? (Not sure whether there are other phases have the
> >> Same issue?)
> >>
> >> Let me know if you have any suggestion.
> >>
> >> Thanks a lot for your help.
> >>
> >> Qing
> >>
> >> --
> >> Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
> >> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> >> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
> >>
> >>
> >
> > --
> > Richard Biener <rguenther@suse.de>
> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> > Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Richard Biener]

On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski <pinskia@gmail.com> wrote:
>On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
><gcc-patches@gcc.gnu.org> wrote:
>>
>> I came up with a very simple testing case that can repeat the same
>issue:
>>
>> [qinzhao@localhost gcc]$ cat t.c
>> extern void bar (int);
>> void foo (int a)
>> {
>>   int i;
>>   for (i = 0; i < a; i++) {
>>     if (__extension__({int size2;
>>         size2 = 4;
>>         size2 > 5;}))
>>     bar (a);
>>   }
>> }
>
>You should show the full dump,
>What we have is the following:
>
>
>
>size2_3 = PHI <size2_1(D), size2_13>
>   <bb 3> :
>
>size2_12 = .DEFERRED_INIT (size2_3, 2);
>size2_13 = 4;
>
>So CCP decides to propagate 4 into the PHI and then decides size2_1(D)
>is undefined so size2_3 is then considered 4 and propagates it into
>the .DEFERRED_INIT.

Which means the DEFERED_INIT is inserted at the wrong place. 

Richard. 
>
>Thanks,
>Andrew
>
>
>>
>> [qinzhao@localhost gcc]$
>/home/qinzhao/Work/GCC/gcc_build_debug/gcc/xgcc
>-B/home/qinzhao/Work/GCC/gcc_build_debug/gcc/ -std=c99   -m64 
>-march=native -ftrivial-auto-var-init=zero t.c -fdump-tree-all  -O1
>> t.c: In function ‘foo’:
>> t.c:11:1: error: ‘DEFFERED_INIT’ calls should have the same LHS as
>the first argument
>>    11 | }
>>       | ^
>> size2_12 = .DEFERRED_INIT (4, 2);
>> during GIMPLE pass: ccp
>> dump file: a-t.c.032t.ccp1
>> t.c:11:1: internal compiler error: verify_gimple failed
>> 0x143ee47 verify_gimple_in_cfg(function*, bool)
>>         ../../latest_gcc/gcc/tree-cfg.c:5501
>> 0x122d799 execute_function_todo
>>         ../../latest_gcc/gcc/passes.c:2042
>> 0x122c74b do_per_function
>>         ../../latest_gcc/gcc/passes.c:1687
>> 0x122d986 execute_todo
>>         ../../latest_gcc/gcc/passes.c:2096
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
>> Please include the complete backtrace with any bug report.
>> See <https://gcc.gnu.org/bugs/> for instructions.
>>
>> In this testing case, both “I” and “size2” are auto vars that are not
>initialized at declaration but are initialized later by assignment.
>> However, “I” doesn’t have any issue, but “size2” has such issue.
>>
>> ******“ssa” dump:
>>
>>   <bb 2> :
>>   i_7 = .DEFERRED_INIT (i_6(D), 2);
>>   i_8 = 0;
>>   goto <bb 6>; [INV]
>>
>>   <bb 3> :
>>   size2_12 = .DEFERRED_INIT (size2_3, 2);
>>   size2_13 = 4;
>>
>> ******”ccp1” dump:
>>
>>   <bb 2> :
>>   i_7 = .DEFERRED_INIT (i_6(D), 2);
>>   goto <bb 4>; [INV]
>>
>>   <bb 3> :
>>   size2_12 = .DEFERRED_INIT (4, 2);
>>
>> So, I am wondering:  Is it possible that “ssa” phase have a bug ?
>>
>> Qing
>>
>> > On Jun 30, 2021, at 9:39 AM, Richard Biener <rguenther@suse.de>
>wrote:
>> >
>> > On Wed, 30 Jun 2021, Qing Zhao wrote:
>> >
>> >>
>> >>
>> >> On Jun 30, 2021, at 2:46 AM, Richard Biener
><rguenther@suse.de<mailto:rguenther@suse.de>> wrote:
>> >>
>> >> On Wed, 30 Jun 2021, Qing Zhao wrote:
>> >>
>> >> Hi,
>> >>
>> >> I am testing the 4th patch of -ftrivial-auto-var-init with CPU2017
>today, and found the following issues:
>> >>
>> >> ****In the dump file of “*t.i.031t.objsz1”, we have:
>> >>
>> >> <bb 3> :
>> >> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>> >> __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);
>> >>
>> >> I looks like this .DEFERRED_INIT initializes an already
>initialized
>> >> variable.
>> >>
>> >> Yes.
>> >>
>> >> For cases like the following:
>> >>
>> >> int s2_len;
>> >> s2_len = 4;
>> >>
>> >> i.e, the initialization is not at the declaration.
>> >>
>> >> We cannot avoid initialization for such cases.
>> >
>> > But I'd have expected
>> >
>> >  s2_len = .DEFERRED_INIT (s2_len, 0);
>> >  s2_len = 4;
>> >
>> > from the above - thus the deferred init _before_ the first
>> > "use" which is the explicit init.  How does the other order
>> > happen to materialize?  As said, I believe it shouldn't.
>> >
>> >> I'd expect to only ever see default definition SSA names
>> >> as first argument to .DEFERRED_INIT.
>> >>
>> >> You mean something like:
>> >> __s2_len_218 = .DEFERRED_INIT (__s2_len, 2);
>> >
>> > No,
>> >
>> > __s2_len_218 = .DEFERRED_INIT (__s2_len_217(D), 2);
>> >
>> >> ?
>> >>
>> >>
>> >> __s2_len_219 = 7;
>> >> if (__s2_len_219 <= 3)
>> >>   goto <bb 4>; [INV]
>> >> else
>> >>   goto <bb 9>; [INV]
>> >>
>> >> <bb 4> :
>> >> _1 = (long unsigned int) i_175;
>> >>
>> >>
>> >> ****However, after “ccp”, in “t.i.032t.ccp1”, we have:
>> >>
>> >> <bb 3> :
>> >> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>> >> __s2_len_218 = .DEFERRED_INIT (7, 2);
>> >> _36 = (long unsigned int) i_175;
>> >> _37 = _36 * 8;
>> >> _38 = argv_220(D) + _37;
>> >>
>> >>
>> >> Looks like that the optimization “ccp” replaced the first argument
>of the call .DEFERRED_INIT with the constant 7.
>> >> This should be avoided.
>> >>
>> >> (NOTE, this issue existed in the previous patches, however, only
>exposed with this version since I added more verification
>> >> code in tree-cfg.c to verify the call to .DEFERRED_INIT).
>> >>
>> >> I am wondering what’s the best solution to this problem?
>> >>
>> >> I think you have to trace where this "bogus" .DEFERRED_INIT comes
>from
>> >> originally.  Or alternatively, if this is unavoidable,
>> >>
>> >> This is unavoidable, I believe.
>> >
>> > I see but don't believe it yet ;)
>> >
>> >> add "constant
>> >> folding" of .DEFERRED_INIT so that defered init of an initialized
>> >> object becomes the object itself, thus retain the previous -
>eventually
>> >> partial - initialization only.
>> >>
>> >> If this additional .DEFERRED_INIT will be kept till RTL expansion
>phase, then it will become a real initialization:
>> >>
>> >> i.e.
>> >>
>> >> s2_len = 0;    //.DEFERRED_INIT expanded
>> >> s2_len = 4;    // the original initialization
>> >>
>> >> Then the first initialization will be eliminated by current RTL
>optimization easily, right?
>> >
>> > Well, in your example above it's effectively elimiated by GIMPLE
>> > optimization.  IIRC you're using the first argument of
>.DEFERRED_INIT
>> > for diagnostic purposes only, correct?
>> >
>> > Richard.
>> >
>> >> Qing
>> >>
>> >>
>> >> Richard.
>> >>
>> >> Can we add any attribute to the internal function argument to
>prevent later optimizations that might applied on it?
>> >> Or just update “ccp” phase to specially handle calls to
>.DEFERRED_INIT? (Not sure whether there are other phases have the
>> >> Same issue?)
>> >>
>> >> Let me know if you have any suggestion.
>> >>
>> >> Thanks a lot for your help.
>> >>
>> >> Qing
>> >>
>> >> --
>> >> Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
>> >> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
>Nuernberg,
>> >> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>> >>
>> >>
>> >
>> > --
>> > Richard Biener <rguenther@suse.de>
>> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
>Nuernberg,
>> > Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

Hi, Andrew,

Thanks a lot for your explanation.

On Jun 30, 2021, at 12:20 PM, Andrew Pinski <pinskia@gmail.com<mailto:pinskia@gmail.com>> wrote:

On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
<gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>> wrote:

I came up with a very simple testing case that can repeat the same issue:

[qinzhao@localhost gcc]$ cat t.c
extern void bar (int);
void foo (int a)
{
 int i;
 for (i = 0; i < a; i++) {
   if (__extension__({int size2;
       size2 = 4;
       size2 > 5;}))
   bar (a);
 }
}

You should show the full dump,

For the above small testing case:

*****The full dump of “gimple” phase is:

void foo (int a)
{
  int D.2240;
  int i;

  i = .DEFERRED_INIT (i, 2);
  i = 0;
  goto <D.2244>;
  <D.2243>:
  {
    int size2;

    size2 = .DEFERRED_INIT (size2, 2);
    size2 = 4;
    _1 = size2 > 5;
    D.2240 = (int) _1;
  }
  if (D.2240 != 0) goto <D.2246>; else goto <D.2247>;
  <D.2246>:
  bar (a);
  <D.2247>:
  i = i + 1;
  <D.2244>:
  if (i < a) goto <D.2243>; else goto <D.2241>;
  <D.2241>:
}

*****The full dump of “ssa” phase is:


;; Function foo (foo, funcdef_no=0, decl_uid=2236, cgraph_uid=1, symbol_order=0)

void foo (int a)
{
  int size2;
  int i;
  _Bool _1;
  int _14;

  <bb 2> :
  i_7 = .DEFERRED_INIT (i_6(D), 2);
  i_8 = 0;
  goto <bb 6>; [INV]

  <bb 3> :
  size2_12 = .DEFERRED_INIT (size2_3, 2);
  size2_13 = 4;
  _1 = size2_13 > 5;
  _14 = (int) _1;
  if (_14 != 0)
    goto <bb 4>; [INV]
  else
    goto <bb 5>; [INV]

  <bb 4> :
  bar (a_11(D));

  <bb 5> :
  i_16 = i_2 + 1;

  <bb 6> :
  # i_2 = PHI <i_8(2), i_16(5)>
  # size2_3 = PHI <size2_9(D)(2), size2_13(5)>
  if (i_2 < a_11(D))
    goto <bb 3>; [INV]
  else
    goto <bb 7>; [INV]

  <bb 7> :
  return;

}


******The full dump of the “ccp1” phase is:


;; Function foo (foo, funcdef_no=0, decl_uid=2236, cgraph_uid=1, symbol_order=0)

Folding predicate 0 != 0 to 0
Removing basic block 4
Merging blocks 3 and 5



EMERGENCY DUMP:

void foo (int a)
{
  int size2;
  int i;

  <bb 2> :
  i_7 = .DEFERRED_INIT (i_6(D), 2);
  goto <bb 4>; [INV]

  <bb 3> :
  size2_12 = .DEFERRED_INIT (4, 2);
  i_16 = i_2 + 1;

  <bb 4> :
  # i_2 = PHI <0(2), i_16(3)>
  if (i_2 < a_11(D))
    goto <bb 3>; [INV]
  else
    goto <bb 5>; [INV]

  <bb 5> :
  return;

}



What we have is the following:

size2_3 = PHI <size2_1(D), size2_13>
  <bb 3> :

size2_12 = .DEFERRED_INIT (size2_3, 2);
size2_13 = 4;

So CCP decides to propagate 4 into the PHI and then decides size2_1(D)
is undefined so size2_3 is then considered 4 and propagates it into
the .DEFERRED_INIT.

Okay, now I understand.

So, both SSA and CCP do correctly?

Is there simple solution to avoid CCP from propagating 4 into .DEFERRED_INIT?

thanks.

Qing

Thanks,
Andrew



[qinzhao@localhost gcc]$ /home/qinzhao/Work/GCC/gcc_build_debug/gcc/xgcc -B/home/qinzhao/Work/GCC/gcc_build_debug/gcc/ -std=c99   -m64  -march=native -ftrivial-auto-var-init=zero t.c -fdump-tree-all  -O1
t.c: In function ‘foo’:
t.c:11:1: error: ‘DEFFERED_INIT’ calls should have the same LHS as the first argument
  11 | }
     | ^
size2_12 = .DEFERRED_INIT (4, 2);
during GIMPLE pass: ccp
dump file: a-t.c.032t.ccp1
t.c:11:1: internal compiler error: verify_gimple failed
0x143ee47 verify_gimple_in_cfg(function*, bool)
       ../../latest_gcc/gcc/tree-cfg.c:5501
0x122d799 execute_function_todo
       ../../latest_gcc/gcc/passes.c:2042
0x122c74b do_per_function
       ../../latest_gcc/gcc/passes.c:1687
0x122d986 execute_todo
       ../../latest_gcc/gcc/passes.c:2096
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

In this testing case, both “I” and “size2” are auto vars that are not initialized at declaration but are initialized later by assignment.
However, “I” doesn’t have any issue, but “size2” has such issue.

******“ssa” dump:

 <bb 2> :
 i_7 = .DEFERRED_INIT (i_6(D), 2);
 i_8 = 0;
 goto <bb 6>; [INV]

 <bb 3> :
 size2_12 = .DEFERRED_INIT (size2_3, 2);
 size2_13 = 4;

******”ccp1” dump:

 <bb 2> :
 i_7 = .DEFERRED_INIT (i_6(D), 2);
 goto <bb 4>; [INV]

 <bb 3> :
 size2_12 = .DEFERRED_INIT (4, 2);

So, I am wondering:  Is it possible that “ssa” phase have a bug ?

Qing

On Jun 30, 2021, at 9:39 AM, Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>> wrote:

On Wed, 30 Jun 2021, Qing Zhao wrote:



On Jun 30, 2021, at 2:46 AM, Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de><mailto:rguenther@suse.de>> wrote:

On Wed, 30 Jun 2021, Qing Zhao wrote:

Hi,

I am testing the 4th patch of -ftrivial-auto-var-init with CPU2017 today, and found the following issues:

****In the dump file of “*t.i.031t.objsz1”, we have:

<bb 3> :
__s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
__s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);

I looks like this .DEFERRED_INIT initializes an already initialized
variable.

Yes.

For cases like the following:

int s2_len;
s2_len = 4;

i.e, the initialization is not at the declaration.

We cannot avoid initialization for such cases.

But I'd have expected

s2_len = .DEFERRED_INIT (s2_len, 0);
s2_len = 4;

from the above - thus the deferred init _before_ the first
"use" which is the explicit init.  How does the other order
happen to materialize?  As said, I believe it shouldn't.

I'd expect to only ever see default definition SSA names
as first argument to .DEFERRED_INIT.

You mean something like:
__s2_len_218 = .DEFERRED_INIT (__s2_len, 2);

No,

__s2_len_218 = .DEFERRED_INIT (__s2_len_217(D), 2);

?


__s2_len_219 = 7;
if (__s2_len_219 <= 3)
 goto <bb 4>; [INV]
else
 goto <bb 9>; [INV]

<bb 4> :
_1 = (long unsigned int) i_175;


****However, after “ccp”, in “t.i.032t.ccp1”, we have:

<bb 3> :
__s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
__s2_len_218 = .DEFERRED_INIT (7, 2);
_36 = (long unsigned int) i_175;
_37 = _36 * 8;
_38 = argv_220(D) + _37;


Looks like that the optimization “ccp” replaced the first argument of the call .DEFERRED_INIT with the constant 7.
This should be avoided.

(NOTE, this issue existed in the previous patches, however, only exposed with this version since I added more verification
code in tree-cfg.c to verify the call to .DEFERRED_INIT).

I am wondering what’s the best solution to this problem?

I think you have to trace where this "bogus" .DEFERRED_INIT comes from
originally.  Or alternatively, if this is unavoidable,

This is unavoidable, I believe.

I see but don't believe it yet ;)

add "constant
folding" of .DEFERRED_INIT so that defered init of an initialized
object becomes the object itself, thus retain the previous - eventually
partial - initialization only.

If this additional .DEFERRED_INIT will be kept till RTL expansion phase, then it will become a real initialization:

i.e.

s2_len = 0;    //.DEFERRED_INIT expanded
s2_len = 4;    // the original initialization

Then the first initialization will be eliminated by current RTL optimization easily, right?

Well, in your example above it's effectively elimiated by GIMPLE
optimization.  IIRC you're using the first argument of .DEFERRED_INIT
for diagnostic purposes only, correct?

Richard.

Qing


Richard.

Can we add any attribute to the internal function argument to prevent later optimizations that might applied on it?
Or just update “ccp” phase to specially handle calls to .DEFERRED_INIT? (Not sure whether there are other phases have the
Same issue?)

Let me know if you have any suggestion.

Thanks a lot for your help.

Qing

--
Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de><mailto:rguenther@suse.de>>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)



--
Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de> wrote:
> 
> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>> 
>>> I came up with a very simple testing case that can repeat the same
>> issue:
>>> 
>>> [qinzhao@localhost gcc]$ cat t.c
>>> extern void bar (int);
>>> void foo (int a)
>>> {
>>>  int i;
>>>  for (i = 0; i < a; i++) {
>>>    if (__extension__({int size2;
>>>        size2 = 4;
>>>        size2 > 5;}))
>>>    bar (a);
>>>  }
>>> }
>> 
>> You should show the full dump,
>> What we have is the following:
>> 
>> 
>> 
>> size2_3 = PHI <size2_1(D), size2_13>
>>  <bb 3> :
>> 
>> size2_12 = .DEFERRED_INIT (size2_3, 2);
>> size2_13 = 4;
>> 
>> So CCP decides to propagate 4 into the PHI and then decides size2_1(D)
>> is undefined so size2_3 is then considered 4 and propagates it into
>> the .DEFERRED_INIT.
> 
> Which means the DEFERED_INIT is inserted at the wrong place. 

Then, where is the correct place for “.DEFERRED_INIT(size2,2)?

The variable “size2” is a block scope variable which is declared inside the “if” condition:

>>> if (__extension__({int size2;
>>>        size2 = 4;
>>>        size2 > 5;}))

So, it’s reasonable to insert the initialization inside this block and immediately after the declaration, This is what the patch currently does:

*****The full dump of “gimple” phase is:

void foo (int a)
{
 int D.2240;
 int i;

 i = .DEFERRED_INIT (i, 2);
 i = 0;
 goto <D.2244>;
 <D.2243>:
 {
   int size2;

   size2 = .DEFERRED_INIT (size2, 2);
   size2 = 4;
   _1 = size2 > 5;
   D.2240 = (int) _1;
 }
 if (D.2240 != 0) goto <D.2246>; else goto <D.2247>;
 <D.2246>:
 bar (a);
 <D.2247>:
 i = i + 1;
 <D.2244>:
 if (i < a) goto <D.2243>; else goto <D.2241>;
 <D.2241>:
}

However, I suspect that the SSA phase moved the “size2” out of its block scope as following:

******The full “SSA” dump is:

;; Function foo (foo, funcdef_no=0, decl_uid=2236, cgraph_uid=1, symbol_order=0)

void foo (int a)
{
 int size2;
 int i;
 _Bool _1;
 int _14;

 <bb 2> :
 i_7 = .DEFERRED_INIT (i_6(D), 2);
 i_8 = 0;
 goto <bb 6>; [INV]

 <bb 3> :
 size2_12 = .DEFERRED_INIT (size2_3, 2);
 size2_13 = 4;
 _1 = size2_13 > 5;
 _14 = (int) _1;
 if (_14 != 0)
   goto <bb 4>; [INV]
 else
   goto <bb 5>; [INV]

 <bb 4> :
 bar (a_11(D));

 <bb 5> :
 i_16 = i_2 + 1;

 <bb 6> :
 # i_2 = PHI <i_8(2), i_16(5)>
 # size2_3 = PHI <size2_9(D)(2), size2_13(5)>
 if (i_2 < a_11(D))
   goto <bb 3>; [INV]
 else
   goto <bb 7>; [INV]

 <bb 7> :
 return;

}

In the above, we can see that “ # size2_3 = PHI <size2_9(D)(2), size2_13(5)>”  is outside of its block scope already.
“size2” should not be in the same scope as “I" . 

This looks incorrect SSA transformation to me.

What do you think?

Qing

> 
> Richard. 
>> 
>> Thanks,
>> Andrew
>> 
>> 
>>> 
>>> [qinzhao@localhost gcc]$
>> /home/qinzhao/Work/GCC/gcc_build_debug/gcc/xgcc
>> -B/home/qinzhao/Work/GCC/gcc_build_debug/gcc/ -std=c99   -m64 
>> -march=native -ftrivial-auto-var-init=zero t.c -fdump-tree-all  -O1
>>> t.c: In function ‘foo’:
>>> t.c:11:1: error: ‘DEFFERED_INIT’ calls should have the same LHS as
>> the first argument
>>>   11 | }
>>>      | ^
>>> size2_12 = .DEFERRED_INIT (4, 2);
>>> during GIMPLE pass: ccp
>>> dump file: a-t.c.032t.ccp1
>>> t.c:11:1: internal compiler error: verify_gimple failed
>>> 0x143ee47 verify_gimple_in_cfg(function*, bool)
>>>        ../../latest_gcc/gcc/tree-cfg.c:5501
>>> 0x122d799 execute_function_todo
>>>        ../../latest_gcc/gcc/passes.c:2042
>>> 0x122c74b do_per_function
>>>        ../../latest_gcc/gcc/passes.c:1687
>>> 0x122d986 execute_todo
>>>        ../../latest_gcc/gcc/passes.c:2096
>>> Please submit a full bug report,
>>> with preprocessed source if appropriate.
>>> Please include the complete backtrace with any bug report.
>>> See <https://gcc.gnu.org/bugs/> for instructions.
>>> 
>>> In this testing case, both “I” and “size2” are auto vars that are not
>> initialized at declaration but are initialized later by assignment.
>>> However, “I” doesn’t have any issue, but “size2” has such issue.
>>> 
>>> ******“ssa” dump:
>>> 
>>>  <bb 2> :
>>>  i_7 = .DEFERRED_INIT (i_6(D), 2);
>>>  i_8 = 0;
>>>  goto <bb 6>; [INV]
>>> 
>>>  <bb 3> :
>>>  size2_12 = .DEFERRED_INIT (size2_3, 2);
>>>  size2_13 = 4;
>>> 
>>> ******”ccp1” dump:
>>> 
>>>  <bb 2> :
>>>  i_7 = .DEFERRED_INIT (i_6(D), 2);
>>>  goto <bb 4>; [INV]
>>> 
>>>  <bb 3> :
>>>  size2_12 = .DEFERRED_INIT (4, 2);
>>> 
>>> So, I am wondering:  Is it possible that “ssa” phase have a bug ?
>>> 
>>> Qing
>>> 
>>>> On Jun 30, 2021, at 9:39 AM, Richard Biener <rguenther@suse.de>
>> wrote:
>>>> 
>>>> On Wed, 30 Jun 2021, Qing Zhao wrote:
>>>> 
>>>>> 
>>>>> 
>>>>> On Jun 30, 2021, at 2:46 AM, Richard Biener
>> <rguenther@suse.de<mailto:rguenther@suse.de>> wrote:
>>>>> 
>>>>> On Wed, 30 Jun 2021, Qing Zhao wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> I am testing the 4th patch of -ftrivial-auto-var-init with CPU2017
>> today, and found the following issues:
>>>>> 
>>>>> ****In the dump file of “*t.i.031t.objsz1”, we have:
>>>>> 
>>>>> <bb 3> :
>>>>> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>>>>> __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);
>>>>> 
>>>>> I looks like this .DEFERRED_INIT initializes an already
>> initialized
>>>>> variable.
>>>>> 
>>>>> Yes.
>>>>> 
>>>>> For cases like the following:
>>>>> 
>>>>> int s2_len;
>>>>> s2_len = 4;
>>>>> 
>>>>> i.e, the initialization is not at the declaration.
>>>>> 
>>>>> We cannot avoid initialization for such cases.
>>>> 
>>>> But I'd have expected
>>>> 
>>>> s2_len = .DEFERRED_INIT (s2_len, 0);
>>>> s2_len = 4;
>>>> 
>>>> from the above - thus the deferred init _before_ the first
>>>> "use" which is the explicit init.  How does the other order
>>>> happen to materialize?  As said, I believe it shouldn't.
>>>> 
>>>>> I'd expect to only ever see default definition SSA names
>>>>> as first argument to .DEFERRED_INIT.
>>>>> 
>>>>> You mean something like:
>>>>> __s2_len_218 = .DEFERRED_INIT (__s2_len, 2);
>>>> 
>>>> No,
>>>> 
>>>> __s2_len_218 = .DEFERRED_INIT (__s2_len_217(D), 2);
>>>> 
>>>>> ?
>>>>> 
>>>>> 
>>>>> __s2_len_219 = 7;
>>>>> if (__s2_len_219 <= 3)
>>>>>  goto <bb 4>; [INV]
>>>>> else
>>>>>  goto <bb 9>; [INV]
>>>>> 
>>>>> <bb 4> :
>>>>> _1 = (long unsigned int) i_175;
>>>>> 
>>>>> 
>>>>> ****However, after “ccp”, in “t.i.032t.ccp1”, we have:
>>>>> 
>>>>> <bb 3> :
>>>>> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>>>>> __s2_len_218 = .DEFERRED_INIT (7, 2);
>>>>> _36 = (long unsigned int) i_175;
>>>>> _37 = _36 * 8;
>>>>> _38 = argv_220(D) + _37;
>>>>> 
>>>>> 
>>>>> Looks like that the optimization “ccp” replaced the first argument
>> of the call .DEFERRED_INIT with the constant 7.
>>>>> This should be avoided.
>>>>> 
>>>>> (NOTE, this issue existed in the previous patches, however, only
>> exposed with this version since I added more verification
>>>>> code in tree-cfg.c to verify the call to .DEFERRED_INIT).
>>>>> 
>>>>> I am wondering what’s the best solution to this problem?
>>>>> 
>>>>> I think you have to trace where this "bogus" .DEFERRED_INIT comes
>> from
>>>>> originally.  Or alternatively, if this is unavoidable,
>>>>> 
>>>>> This is unavoidable, I believe.
>>>> 
>>>> I see but don't believe it yet ;)
>>>> 
>>>>> add "constant
>>>>> folding" of .DEFERRED_INIT so that defered init of an initialized
>>>>> object becomes the object itself, thus retain the previous -
>> eventually
>>>>> partial - initialization only.
>>>>> 
>>>>> If this additional .DEFERRED_INIT will be kept till RTL expansion
>> phase, then it will become a real initialization:
>>>>> 
>>>>> i.e.
>>>>> 
>>>>> s2_len = 0;    //.DEFERRED_INIT expanded
>>>>> s2_len = 4;    // the original initialization
>>>>> 
>>>>> Then the first initialization will be eliminated by current RTL
>> optimization easily, right?
>>>> 
>>>> Well, in your example above it's effectively elimiated by GIMPLE
>>>> optimization.  IIRC you're using the first argument of
>> .DEFERRED_INIT
>>>> for diagnostic purposes only, correct?
>>>> 
>>>> Richard.
>>>> 
>>>>> Qing
>>>>> 
>>>>> 
>>>>> Richard.
>>>>> 
>>>>> Can we add any attribute to the internal function argument to
>> prevent later optimizations that might applied on it?
>>>>> Or just update “ccp” phase to specially handle calls to
>> .DEFERRED_INIT? (Not sure whether there are other phases have the
>>>>> Same issue?)
>>>>> 
>>>>> Let me know if you have any suggestion.
>>>>> 
>>>>> Thanks a lot for your help.
>>>>> 
>>>>> Qing
>>>>> 
>>>>> --
>>>>> Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
>>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
>> Nuernberg,
>>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>>>> 
>>>>> 
>>>> 
>>>> --
>>>> Richard Biener <rguenther@suse.de>
>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
>> Nuernberg,
>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>> 
> 

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Richard Biener]

On June 30, 2021 8:07:43 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
>
>
>> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de>
>wrote:
>> 
>> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski
><pinskia@gmail.com> wrote:
>>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> wrote:
>>>> 
>>>> I came up with a very simple testing case that can repeat the same
>>> issue:
>>>> 
>>>> [qinzhao@localhost gcc]$ cat t.c
>>>> extern void bar (int);
>>>> void foo (int a)
>>>> {
>>>>  int i;
>>>>  for (i = 0; i < a; i++) {
>>>>    if (__extension__({int size2;
>>>>        size2 = 4;
>>>>        size2 > 5;}))
>>>>    bar (a);
>>>>  }
>>>> }
>>> 
>>> You should show the full dump,
>>> What we have is the following:
>>> 
>>> 
>>> 
>>> size2_3 = PHI <size2_1(D), size2_13>
>>>  <bb 3> :
>>> 
>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
>>> size2_13 = 4;
>>> 
>>> So CCP decides to propagate 4 into the PHI and then decides
>size2_1(D)
>>> is undefined so size2_3 is then considered 4 and propagates it into
>>> the .DEFERRED_INIT.
>> 
>> Which means the DEFERED_INIT is inserted at the wrong place. 
>
>Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
>
>The variable “size2” is a block scope variable which is declared inside
>the “if” condition:

But that's obviously not how it behaves
During into SSA phase since we're inserting a PHI for it - and we're inserting it because of the use in the DEFERED_INIT call. I suppose you need to fiddle with the SSA rewrite and avoid treating the use as a use but only for the purpose of inserting PHIs...

You might be able to construct a testcase which has a use before the real init where then the optimistic CCP propagation will defeat the DEFERED_INIT otherwise. 

I'd need to play with the actual patch to find a good solution to this problem. 

Richard.

>>>> if (__extension__({int size2;
>>>>        size2 = 4;
>>>>        size2 > 5;}))
>
>So, it’s reasonable to insert the initialization inside this block and
>immediately after the declaration, This is what the patch currently
>does:
>
>*****The full dump of “gimple” phase is:
>
>void foo (int a)
>{
> int D.2240;
> int i;
>
> i = .DEFERRED_INIT (i, 2);
> i = 0;
> goto <D.2244>;
> <D.2243>:
> {
>   int size2;
>
>   size2 = .DEFERRED_INIT (size2, 2);
>   size2 = 4;
>   _1 = size2 > 5;
>   D.2240 = (int) _1;
> }
> if (D.2240 != 0) goto <D.2246>; else goto <D.2247>;
> <D.2246>:
> bar (a);
> <D.2247>:
> i = i + 1;
> <D.2244>:
> if (i < a) goto <D.2243>; else goto <D.2241>;
> <D.2241>:
>}
>
>However, I suspect that the SSA phase moved the “size2” out of its
>block scope as following:
>
>******The full “SSA” dump is:
>
>;; Function foo (foo, funcdef_no=0, decl_uid=2236, cgraph_uid=1,
>symbol_order=0)
>
>void foo (int a)
>{
> int size2;
> int i;
> _Bool _1;
> int _14;
>
> <bb 2> :
> i_7 = .DEFERRED_INIT (i_6(D), 2);
> i_8 = 0;
> goto <bb 6>; [INV]
>
> <bb 3> :
> size2_12 = .DEFERRED_INIT (size2_3, 2);
> size2_13 = 4;
> _1 = size2_13 > 5;
> _14 = (int) _1;
> if (_14 != 0)
>   goto <bb 4>; [INV]
> else
>   goto <bb 5>; [INV]
>
> <bb 4> :
> bar (a_11(D));
>
> <bb 5> :
> i_16 = i_2 + 1;
>
> <bb 6> :
> # i_2 = PHI <i_8(2), i_16(5)>
> # size2_3 = PHI <size2_9(D)(2), size2_13(5)>
> if (i_2 < a_11(D))
>   goto <bb 3>; [INV]
> else
>   goto <bb 7>; [INV]
>
> <bb 7> :
> return;
>
>}
>
>In the above, we can see that “ # size2_3 = PHI <size2_9(D)(2),
>size2_13(5)>”  is outside of its block scope already.
>“size2” should not be in the same scope as “I" . 
>
>This looks incorrect SSA transformation to me.
>
>What do you think?
>
>Qing
>
>> 
>> Richard. 
>>> 
>>> Thanks,
>>> Andrew
>>> 
>>> 
>>>> 
>>>> [qinzhao@localhost gcc]$
>>> /home/qinzhao/Work/GCC/gcc_build_debug/gcc/xgcc
>>> -B/home/qinzhao/Work/GCC/gcc_build_debug/gcc/ -std=c99   -m64 
>>> -march=native -ftrivial-auto-var-init=zero t.c -fdump-tree-all  -O1
>>>> t.c: In function ‘foo’:
>>>> t.c:11:1: error: ‘DEFFERED_INIT’ calls should have the same LHS as
>>> the first argument
>>>>   11 | }
>>>>      | ^
>>>> size2_12 = .DEFERRED_INIT (4, 2);
>>>> during GIMPLE pass: ccp
>>>> dump file: a-t.c.032t.ccp1
>>>> t.c:11:1: internal compiler error: verify_gimple failed
>>>> 0x143ee47 verify_gimple_in_cfg(function*, bool)
>>>>        ../../latest_gcc/gcc/tree-cfg.c:5501
>>>> 0x122d799 execute_function_todo
>>>>        ../../latest_gcc/gcc/passes.c:2042
>>>> 0x122c74b do_per_function
>>>>        ../../latest_gcc/gcc/passes.c:1687
>>>> 0x122d986 execute_todo
>>>>        ../../latest_gcc/gcc/passes.c:2096
>>>> Please submit a full bug report,
>>>> with preprocessed source if appropriate.
>>>> Please include the complete backtrace with any bug report.
>>>> See <https://gcc.gnu.org/bugs/> for instructions.
>>>> 
>>>> In this testing case, both “I” and “size2” are auto vars that are
>not
>>> initialized at declaration but are initialized later by assignment.
>>>> However, “I” doesn’t have any issue, but “size2” has such issue.
>>>> 
>>>> ******“ssa” dump:
>>>> 
>>>>  <bb 2> :
>>>>  i_7 = .DEFERRED_INIT (i_6(D), 2);
>>>>  i_8 = 0;
>>>>  goto <bb 6>; [INV]
>>>> 
>>>>  <bb 3> :
>>>>  size2_12 = .DEFERRED_INIT (size2_3, 2);
>>>>  size2_13 = 4;
>>>> 
>>>> ******”ccp1” dump:
>>>> 
>>>>  <bb 2> :
>>>>  i_7 = .DEFERRED_INIT (i_6(D), 2);
>>>>  goto <bb 4>; [INV]
>>>> 
>>>>  <bb 3> :
>>>>  size2_12 = .DEFERRED_INIT (4, 2);
>>>> 
>>>> So, I am wondering:  Is it possible that “ssa” phase have a bug ?
>>>> 
>>>> Qing
>>>> 
>>>>> On Jun 30, 2021, at 9:39 AM, Richard Biener <rguenther@suse.de>
>>> wrote:
>>>>> 
>>>>> On Wed, 30 Jun 2021, Qing Zhao wrote:
>>>>> 
>>>>>> 
>>>>>> 
>>>>>> On Jun 30, 2021, at 2:46 AM, Richard Biener
>>> <rguenther@suse.de<mailto:rguenther@suse.de>> wrote:
>>>>>> 
>>>>>> On Wed, 30 Jun 2021, Qing Zhao wrote:
>>>>>> 
>>>>>> Hi,
>>>>>> 
>>>>>> I am testing the 4th patch of -ftrivial-auto-var-init with
>CPU2017
>>> today, and found the following issues:
>>>>>> 
>>>>>> ****In the dump file of “*t.i.031t.objsz1”, we have:
>>>>>> 
>>>>>> <bb 3> :
>>>>>> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>>>>>> __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);
>>>>>> 
>>>>>> I looks like this .DEFERRED_INIT initializes an already
>>> initialized
>>>>>> variable.
>>>>>> 
>>>>>> Yes.
>>>>>> 
>>>>>> For cases like the following:
>>>>>> 
>>>>>> int s2_len;
>>>>>> s2_len = 4;
>>>>>> 
>>>>>> i.e, the initialization is not at the declaration.
>>>>>> 
>>>>>> We cannot avoid initialization for such cases.
>>>>> 
>>>>> But I'd have expected
>>>>> 
>>>>> s2_len = .DEFERRED_INIT (s2_len, 0);
>>>>> s2_len = 4;
>>>>> 
>>>>> from the above - thus the deferred init _before_ the first
>>>>> "use" which is the explicit init.  How does the other order
>>>>> happen to materialize?  As said, I believe it shouldn't.
>>>>> 
>>>>>> I'd expect to only ever see default definition SSA names
>>>>>> as first argument to .DEFERRED_INIT.
>>>>>> 
>>>>>> You mean something like:
>>>>>> __s2_len_218 = .DEFERRED_INIT (__s2_len, 2);
>>>>> 
>>>>> No,
>>>>> 
>>>>> __s2_len_218 = .DEFERRED_INIT (__s2_len_217(D), 2);
>>>>> 
>>>>>> ?
>>>>>> 
>>>>>> 
>>>>>> __s2_len_219 = 7;
>>>>>> if (__s2_len_219 <= 3)
>>>>>>  goto <bb 4>; [INV]
>>>>>> else
>>>>>>  goto <bb 9>; [INV]
>>>>>> 
>>>>>> <bb 4> :
>>>>>> _1 = (long unsigned int) i_175;
>>>>>> 
>>>>>> 
>>>>>> ****However, after “ccp”, in “t.i.032t.ccp1”, we have:
>>>>>> 
>>>>>> <bb 3> :
>>>>>> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>>>>>> __s2_len_218 = .DEFERRED_INIT (7, 2);
>>>>>> _36 = (long unsigned int) i_175;
>>>>>> _37 = _36 * 8;
>>>>>> _38 = argv_220(D) + _37;
>>>>>> 
>>>>>> 
>>>>>> Looks like that the optimization “ccp” replaced the first
>argument
>>> of the call .DEFERRED_INIT with the constant 7.
>>>>>> This should be avoided.
>>>>>> 
>>>>>> (NOTE, this issue existed in the previous patches, however, only
>>> exposed with this version since I added more verification
>>>>>> code in tree-cfg.c to verify the call to .DEFERRED_INIT).
>>>>>> 
>>>>>> I am wondering what’s the best solution to this problem?
>>>>>> 
>>>>>> I think you have to trace where this "bogus" .DEFERRED_INIT comes
>>> from
>>>>>> originally.  Or alternatively, if this is unavoidable,
>>>>>> 
>>>>>> This is unavoidable, I believe.
>>>>> 
>>>>> I see but don't believe it yet ;)
>>>>> 
>>>>>> add "constant
>>>>>> folding" of .DEFERRED_INIT so that defered init of an initialized
>>>>>> object becomes the object itself, thus retain the previous -
>>> eventually
>>>>>> partial - initialization only.
>>>>>> 
>>>>>> If this additional .DEFERRED_INIT will be kept till RTL expansion
>>> phase, then it will become a real initialization:
>>>>>> 
>>>>>> i.e.
>>>>>> 
>>>>>> s2_len = 0;    //.DEFERRED_INIT expanded
>>>>>> s2_len = 4;    // the original initialization
>>>>>> 
>>>>>> Then the first initialization will be eliminated by current RTL
>>> optimization easily, right?
>>>>> 
>>>>> Well, in your example above it's effectively elimiated by GIMPLE
>>>>> optimization.  IIRC you're using the first argument of
>>> .DEFERRED_INIT
>>>>> for diagnostic purposes only, correct?
>>>>> 
>>>>> Richard.
>>>>> 
>>>>>> Qing
>>>>>> 
>>>>>> 
>>>>>> Richard.
>>>>>> 
>>>>>> Can we add any attribute to the internal function argument to
>>> prevent later optimizations that might applied on it?
>>>>>> Or just update “ccp” phase to specially handle calls to
>>> .DEFERRED_INIT? (Not sure whether there are other phases have the
>>>>>> Same issue?)
>>>>>> 
>>>>>> Let me know if you have any suggestion.
>>>>>> 
>>>>>> Thanks a lot for your help.
>>>>>> 
>>>>>> Qing
>>>>>> 
>>>>>> --
>>>>>> Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
>>>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
>>> Nuernberg,
>>>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>>>>> 
>>>>>> 
>>>>> 
>>>>> --
>>>>> Richard Biener <rguenther@suse.de>
>>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
>>> Nuernberg,
>>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>>> 
>> 

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

Hi,

I came up with the following small testing case and compile it WITHOUT -ftrivial-auto-var-init option, the same problem:

[qinzhao@localhost gcc]$ cat t1.c
extern void bar (int);
extern int ART_INIT(int, int);
void foo (int a)
{
  int i;
  for (i = 0; i < a; i++) {
    if (__extension__({int size2;
size2 = ART_INIT (size2, 2);
size2 = 4;
size2 > 5;}))
    bar (a);
  }
}


With /home/qinzhao/Work/GCC/gcc_build_debug/gcc/xgcc -B/home/qinzhao/Work/GCC/gcc_build_debug/gcc/ -std=c99   -m64  -march=native  t1.c -fdump-tree-all  -O1

*****the full dump for “SSA”:

;; Function foo (foo, funcdef_no=0, decl_uid=2239, cgraph_uid=1, symbol_order=0)

void foo (int a)
{
  int size2;
  int i;
  _Bool _1;
  int _13;

  <bb 2> :
  i_6 = 0;
  goto <bb 6>; [INV]

  <bb 3> :
  size2_11 = ART_INIT (size2_3, 2);
  size2_12 = 4;
  _1 = size2_12 > 5;
  _13 = (int) _1;
  if (_13 != 0)
    goto <bb 4>; [INV]
  else
    goto <bb 5>; [INV]

  <bb 4> :
  bar (a_9(D));

  <bb 5> :
  i_15 = i_2 + 1;

  <bb 6> :
  # i_2 = PHI <i_6(2), i_15(5)>
  # size2_3 = PHI <size2_7(D)(2), size2_12(5)>
  if (i_2 < a_9(D))
    goto <bb 3>; [INV]
  else
    goto <bb 7>; [INV]

  <bb 7> :
  return;

}

In the above, we see that “# size2_3 =PHI <size2_7(D)(2), size2_12(5)>” was moved out of the block scope for size2.

******Therefore, after “CCP1”:

;; Function foo (foo, funcdef_no=0, decl_uid=2239, cgraph_uid=1, symbol_order=0)

Folding predicate 0 != 0 to 0
Removing basic block 4
Merging blocks 3 and 5
void foo (int a)
{
  int size2;
  int i;

  <bb 2> :
  goto <bb 4>; [INV]

  <bb 3> :
  size2_11 = ART_INIT (4, 2);
  i_15 = i_2 + 1;

  <bb 4> :
  # i_2 = PHI <0(2), i_15(3)>
  if (i_2 < a_9(D))
    goto <bb 3>; [INV]
  else
    goto <bb 5>; [INV]

  <bb 5> :
  return;

}

The constant “4” is propagated to the call to “ART_INIT”.

This looks incorrect. Right?

Qing

On Jun 30, 2021, at 1:07 PM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>> wrote:



On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>> wrote:

On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski <pinskia@gmail.com<mailto:pinskia@gmail.com>> wrote:
On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
<gcc-patches@gcc.gnu.org<mailto:gcc-patches@gcc.gnu.org>> wrote:

I came up with a very simple testing case that can repeat the same
issue:

[qinzhao@localhost gcc]$ cat t.c
extern void bar (int);
void foo (int a)
{
int i;
for (i = 0; i < a; i++) {
  if (__extension__({int size2;
      size2 = 4;
      size2 > 5;}))
  bar (a);
}
}

You should show the full dump,
What we have is the following:



size2_3 = PHI <size2_1(D), size2_13>
<bb 3> :

size2_12 = .DEFERRED_INIT (size2_3, 2);
size2_13 = 4;

So CCP decides to propagate 4 into the PHI and then decides size2_1(D)
is undefined so size2_3 is then considered 4 and propagates it into
the .DEFERRED_INIT.

Which means the DEFERED_INIT is inserted at the wrong place.

Then, where is the correct place for “.DEFERRED_INIT(size2,2)?

The variable “size2” is a block scope variable which is declared inside the “if” condition:

if (__extension__({int size2;
      size2 = 4;
      size2 > 5;}))

So, it’s reasonable to insert the initialization inside this block and immediately after the declaration, This is what the patch currently does:

*****The full dump of “gimple” phase is:

void foo (int a)
{
int D.2240;
int i;

i = .DEFERRED_INIT (i, 2);
i = 0;
goto <D.2244>;
<D.2243>:
{
  int size2;

  size2 = .DEFERRED_INIT (size2, 2);
  size2 = 4;
  _1 = size2 > 5;
  D.2240 = (int) _1;
}
if (D.2240 != 0) goto <D.2246>; else goto <D.2247>;
<D.2246>:
bar (a);
<D.2247>:
i = i + 1;
<D.2244>:
if (i < a) goto <D.2243>; else goto <D.2241>;
<D.2241>:
}

However, I suspect that the SSA phase moved the “size2” out of its block scope as following:

******The full “SSA” dump is:

;; Function foo (foo, funcdef_no=0, decl_uid=2236, cgraph_uid=1, symbol_order=0)

void foo (int a)
{
int size2;
int i;
_Bool _1;
int _14;

<bb 2> :
i_7 = .DEFERRED_INIT (i_6(D), 2);
i_8 = 0;
goto <bb 6>; [INV]

<bb 3> :
size2_12 = .DEFERRED_INIT (size2_3, 2);
size2_13 = 4;
_1 = size2_13 > 5;
_14 = (int) _1;
if (_14 != 0)
  goto <bb 4>; [INV]
else
  goto <bb 5>; [INV]

<bb 4> :
bar (a_11(D));

<bb 5> :
i_16 = i_2 + 1;

<bb 6> :
# i_2 = PHI <i_8(2), i_16(5)>
# size2_3 = PHI <size2_9(D)(2), size2_13(5)>
if (i_2 < a_11(D))
  goto <bb 3>; [INV]
else
  goto <bb 7>; [INV]

<bb 7> :
return;

}

In the above, we can see that “ # size2_3 = PHI <size2_9(D)(2), size2_13(5)>”  is outside of its block scope already.
“size2” should not be in the same scope as “I" .

This looks incorrect SSA transformation to me.

What do you think?

Qing


Richard.

Thanks,
Andrew



[qinzhao@localhost gcc]$
/home/qinzhao/Work/GCC/gcc_build_debug/gcc/xgcc
-B/home/qinzhao/Work/GCC/gcc_build_debug/gcc/ -std=c99   -m64
-march=native -ftrivial-auto-var-init=zero t.c -fdump-tree-all  -O1
t.c: In function ‘foo’:
t.c:11:1: error: ‘DEFFERED_INIT’ calls should have the same LHS as
the first argument
 11 | }
    | ^
size2_12 = .DEFERRED_INIT (4, 2);
during GIMPLE pass: ccp
dump file: a-t.c.032t.ccp1
t.c:11:1: internal compiler error: verify_gimple failed
0x143ee47 verify_gimple_in_cfg(function*, bool)
      ../../latest_gcc/gcc/tree-cfg.c:5501
0x122d799 execute_function_todo
      ../../latest_gcc/gcc/passes.c:2042
0x122c74b do_per_function
      ../../latest_gcc/gcc/passes.c:1687
0x122d986 execute_todo
      ../../latest_gcc/gcc/passes.c:2096
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

In this testing case, both “I” and “size2” are auto vars that are not
initialized at declaration but are initialized later by assignment.
However, “I” doesn’t have any issue, but “size2” has such issue.

******“ssa” dump:

<bb 2> :
i_7 = .DEFERRED_INIT (i_6(D), 2);
i_8 = 0;
goto <bb 6>; [INV]

<bb 3> :
size2_12 = .DEFERRED_INIT (size2_3, 2);
size2_13 = 4;

******”ccp1” dump:

<bb 2> :
i_7 = .DEFERRED_INIT (i_6(D), 2);
goto <bb 4>; [INV]

<bb 3> :
size2_12 = .DEFERRED_INIT (4, 2);

So, I am wondering:  Is it possible that “ssa” phase have a bug ?

Qing

On Jun 30, 2021, at 9:39 AM, Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
wrote:

On Wed, 30 Jun 2021, Qing Zhao wrote:



On Jun 30, 2021, at 2:46 AM, Richard Biener
<rguenther@suse.de<mailto:rguenther@suse.de><mailto:rguenther@suse.de>> wrote:

On Wed, 30 Jun 2021, Qing Zhao wrote:

Hi,

I am testing the 4th patch of -ftrivial-auto-var-init with CPU2017
today, and found the following issues:

****In the dump file of “*t.i.031t.objsz1”, we have:

<bb 3> :
__s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
__s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);

I looks like this .DEFERRED_INIT initializes an already
initialized
variable.

Yes.

For cases like the following:

int s2_len;
s2_len = 4;

i.e, the initialization is not at the declaration.

We cannot avoid initialization for such cases.

But I'd have expected

s2_len = .DEFERRED_INIT (s2_len, 0);
s2_len = 4;

from the above - thus the deferred init _before_ the first
"use" which is the explicit init.  How does the other order
happen to materialize?  As said, I believe it shouldn't.

I'd expect to only ever see default definition SSA names
as first argument to .DEFERRED_INIT.

You mean something like:
__s2_len_218 = .DEFERRED_INIT (__s2_len, 2);

No,

__s2_len_218 = .DEFERRED_INIT (__s2_len_217(D), 2);

?


__s2_len_219 = 7;
if (__s2_len_219 <= 3)
goto <bb 4>; [INV]
else
goto <bb 9>; [INV]

<bb 4> :
_1 = (long unsigned int) i_175;


****However, after “ccp”, in “t.i.032t.ccp1”, we have:

<bb 3> :
__s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
__s2_len_218 = .DEFERRED_INIT (7, 2);
_36 = (long unsigned int) i_175;
_37 = _36 * 8;
_38 = argv_220(D) + _37;


Looks like that the optimization “ccp” replaced the first argument
of the call .DEFERRED_INIT with the constant 7.
This should be avoided.

(NOTE, this issue existed in the previous patches, however, only
exposed with this version since I added more verification
code in tree-cfg.c to verify the call to .DEFERRED_INIT).

I am wondering what’s the best solution to this problem?

I think you have to trace where this "bogus" .DEFERRED_INIT comes
from
originally.  Or alternatively, if this is unavoidable,

This is unavoidable, I believe.

I see but don't believe it yet ;)

add "constant
folding" of .DEFERRED_INIT so that defered init of an initialized
object becomes the object itself, thus retain the previous -
eventually
partial - initialization only.

If this additional .DEFERRED_INIT will be kept till RTL expansion
phase, then it will become a real initialization:

i.e.

s2_len = 0;    //.DEFERRED_INIT expanded
s2_len = 4;    // the original initialization

Then the first initialization will be eliminated by current RTL
optimization easily, right?

Well, in your example above it's effectively elimiated by GIMPLE
optimization.  IIRC you're using the first argument of
.DEFERRED_INIT
for diagnostic purposes only, correct?

Richard.

Qing


Richard.

Can we add any attribute to the internal function argument to
prevent later optimizations that might applied on it?
Or just update “ccp” phase to specially handle calls to
.DEFERRED_INIT? (Not sure whether there are other phases have the
Same issue?)

Let me know if you have any suggestion.

Thanks a lot for your help.

Qing

--
Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de><mailto:rguenther@suse.de>>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)



--
Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

> On Jun 30, 2021, at 1:59 PM, Richard Biener <rguenther@suse.de> wrote:
> 
> On June 30, 2021 8:07:43 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
>> 
>> 
>>> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de>
>> wrote:
>>> 
>>> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski
>> <pinskia@gmail.com> wrote:
>>>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
>>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>> 
>>>>> I came up with a very simple testing case that can repeat the same
>>>> issue:
>>>>> 
>>>>> [qinzhao@localhost gcc]$ cat t.c
>>>>> extern void bar (int);
>>>>> void foo (int a)
>>>>> {
>>>>> int i;
>>>>> for (i = 0; i < a; i++) {
>>>>>   if (__extension__({int size2;
>>>>>       size2 = 4;
>>>>>       size2 > 5;}))
>>>>>   bar (a);
>>>>> }
>>>>> }
>>>> 
>>>> You should show the full dump,
>>>> What we have is the following:
>>>> 
>>>> 
>>>> 
>>>> size2_3 = PHI <size2_1(D), size2_13>
>>>> <bb 3> :
>>>> 
>>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
>>>> size2_13 = 4;
>>>> 
>>>> So CCP decides to propagate 4 into the PHI and then decides
>> size2_1(D)
>>>> is undefined so size2_3 is then considered 4 and propagates it into
>>>> the .DEFERRED_INIT.
>>> 
>>> Which means the DEFERED_INIT is inserted at the wrong place. 
>> 
>> Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
>> 
>> The variable “size2” is a block scope variable which is declared inside
>> the “if” condition:
> 
> But that's obviously not how it behaves
> During into SSA phase since we're inserting a PHI for it - and we're inserting it because of the use in the DEFERED_INIT call. I suppose you need to fiddle with the SSA rewrite and avoid treating the use as a use but only for the purpose of inserting PHIs...
> 
> You might be able to construct a testcase which has a use before the real init where then the optimistic CCP propagation will defeat the DEFERED_INIT otherwise. 
> 
> I'd need to play with the actual patch to find a good solution to this problem. 

Do you want me send you the current patch (4th version) with email? 

Or I have a branch at 

https://gitlab.com/x86-gcc/gcc.git
users/qinzhao/auto-init/v3

(And I am currently try to change the DEFERRED_INIT call from:

temp = .DEFERRED_INIT (temp/WITH_SIZE_EXPR(temp), init_type)

To:

temp = .DEFERRED_INIT (SIZE_of_temp, init_type)

And this solution successfully hide this CCP issue and also simplify the implementation at the same time.)

Qing
> 
> Richard.
> 
>>>>> if (__extension__({int size2;
>>>>>       size2 = 4;
>>>>>       size2 > 5;}))
>> 
>> So, it’s reasonable to insert the initialization inside this block and
>> immediately after the declaration, This is what the patch currently
>> does:
>> 
>> *****The full dump of “gimple” phase is:
>> 
>> void foo (int a)
>> {
>> int D.2240;
>> int i;
>> 
>> i = .DEFERRED_INIT (i, 2);
>> i = 0;
>> goto <D.2244>;
>> <D.2243>:
>> {
>>  int size2;
>> 
>>  size2 = .DEFERRED_INIT (size2, 2);
>>  size2 = 4;
>>  _1 = size2 > 5;
>>  D.2240 = (int) _1;
>> }
>> if (D.2240 != 0) goto <D.2246>; else goto <D.2247>;
>> <D.2246>:
>> bar (a);
>> <D.2247>:
>> i = i + 1;
>> <D.2244>:
>> if (i < a) goto <D.2243>; else goto <D.2241>;
>> <D.2241>:
>> }
>> 
>> However, I suspect that the SSA phase moved the “size2” out of its
>> block scope as following:
>> 
>> ******The full “SSA” dump is:
>> 
>> ;; Function foo (foo, funcdef_no=0, decl_uid=2236, cgraph_uid=1,
>> symbol_order=0)
>> 
>> void foo (int a)
>> {
>> int size2;
>> int i;
>> _Bool _1;
>> int _14;
>> 
>> <bb 2> :
>> i_7 = .DEFERRED_INIT (i_6(D), 2);
>> i_8 = 0;
>> goto <bb 6>; [INV]
>> 
>> <bb 3> :
>> size2_12 = .DEFERRED_INIT (size2_3, 2);
>> size2_13 = 4;
>> _1 = size2_13 > 5;
>> _14 = (int) _1;
>> if (_14 != 0)
>>  goto <bb 4>; [INV]
>> else
>>  goto <bb 5>; [INV]
>> 
>> <bb 4> :
>> bar (a_11(D));
>> 
>> <bb 5> :
>> i_16 = i_2 + 1;
>> 
>> <bb 6> :
>> # i_2 = PHI <i_8(2), i_16(5)>
>> # size2_3 = PHI <size2_9(D)(2), size2_13(5)>
>> if (i_2 < a_11(D))
>>  goto <bb 3>; [INV]
>> else
>>  goto <bb 7>; [INV]
>> 
>> <bb 7> :
>> return;
>> 
>> }
>> 
>> In the above, we can see that “ # size2_3 = PHI <size2_9(D)(2),
>> size2_13(5)>”  is outside of its block scope already.
>> “size2” should not be in the same scope as “I" . 
>> 
>> This looks incorrect SSA transformation to me.
>> 
>> What do you think?
>> 
>> Qing
>> 
>>> 
>>> Richard. 
>>>> 
>>>> Thanks,
>>>> Andrew
>>>> 
>>>> 
>>>>> 
>>>>> [qinzhao@localhost gcc]$
>>>> /home/qinzhao/Work/GCC/gcc_build_debug/gcc/xgcc
>>>> -B/home/qinzhao/Work/GCC/gcc_build_debug/gcc/ -std=c99   -m64 
>>>> -march=native -ftrivial-auto-var-init=zero t.c -fdump-tree-all  -O1
>>>>> t.c: In function ‘foo’:
>>>>> t.c:11:1: error: ‘DEFFERED_INIT’ calls should have the same LHS as
>>>> the first argument
>>>>>  11 | }
>>>>>     | ^
>>>>> size2_12 = .DEFERRED_INIT (4, 2);
>>>>> during GIMPLE pass: ccp
>>>>> dump file: a-t.c.032t.ccp1
>>>>> t.c:11:1: internal compiler error: verify_gimple failed
>>>>> 0x143ee47 verify_gimple_in_cfg(function*, bool)
>>>>>       ../../latest_gcc/gcc/tree-cfg.c:5501
>>>>> 0x122d799 execute_function_todo
>>>>>       ../../latest_gcc/gcc/passes.c:2042
>>>>> 0x122c74b do_per_function
>>>>>       ../../latest_gcc/gcc/passes.c:1687
>>>>> 0x122d986 execute_todo
>>>>>       ../../latest_gcc/gcc/passes.c:2096
>>>>> Please submit a full bug report,
>>>>> with preprocessed source if appropriate.
>>>>> Please include the complete backtrace with any bug report.
>>>>> See <https://gcc.gnu.org/bugs/> for instructions.
>>>>> 
>>>>> In this testing case, both “I” and “size2” are auto vars that are
>> not
>>>> initialized at declaration but are initialized later by assignment.
>>>>> However, “I” doesn’t have any issue, but “size2” has such issue.
>>>>> 
>>>>> ******“ssa” dump:
>>>>> 
>>>>> <bb 2> :
>>>>> i_7 = .DEFERRED_INIT (i_6(D), 2);
>>>>> i_8 = 0;
>>>>> goto <bb 6>; [INV]
>>>>> 
>>>>> <bb 3> :
>>>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
>>>>> size2_13 = 4;
>>>>> 
>>>>> ******”ccp1” dump:
>>>>> 
>>>>> <bb 2> :
>>>>> i_7 = .DEFERRED_INIT (i_6(D), 2);
>>>>> goto <bb 4>; [INV]
>>>>> 
>>>>> <bb 3> :
>>>>> size2_12 = .DEFERRED_INIT (4, 2);
>>>>> 
>>>>> So, I am wondering:  Is it possible that “ssa” phase have a bug ?
>>>>> 
>>>>> Qing
>>>>> 
>>>>>> On Jun 30, 2021, at 9:39 AM, Richard Biener <rguenther@suse.de>
>>>> wrote:
>>>>>> 
>>>>>> On Wed, 30 Jun 2021, Qing Zhao wrote:
>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> On Jun 30, 2021, at 2:46 AM, Richard Biener
>>>> <rguenther@suse.de<mailto:rguenther@suse.de>> wrote:
>>>>>>> 
>>>>>>> On Wed, 30 Jun 2021, Qing Zhao wrote:
>>>>>>> 
>>>>>>> Hi,
>>>>>>> 
>>>>>>> I am testing the 4th patch of -ftrivial-auto-var-init with
>> CPU2017
>>>> today, and found the following issues:
>>>>>>> 
>>>>>>> ****In the dump file of “*t.i.031t.objsz1”, we have:
>>>>>>> 
>>>>>>> <bb 3> :
>>>>>>> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>>>>>>> __s2_len_218 = .DEFERRED_INIT (__s2_len_177, 2);
>>>>>>> 
>>>>>>> I looks like this .DEFERRED_INIT initializes an already
>>>> initialized
>>>>>>> variable.
>>>>>>> 
>>>>>>> Yes.
>>>>>>> 
>>>>>>> For cases like the following:
>>>>>>> 
>>>>>>> int s2_len;
>>>>>>> s2_len = 4;
>>>>>>> 
>>>>>>> i.e, the initialization is not at the declaration.
>>>>>>> 
>>>>>>> We cannot avoid initialization for such cases.
>>>>>> 
>>>>>> But I'd have expected
>>>>>> 
>>>>>> s2_len = .DEFERRED_INIT (s2_len, 0);
>>>>>> s2_len = 4;
>>>>>> 
>>>>>> from the above - thus the deferred init _before_ the first
>>>>>> "use" which is the explicit init.  How does the other order
>>>>>> happen to materialize?  As said, I believe it shouldn't.
>>>>>> 
>>>>>>> I'd expect to only ever see default definition SSA names
>>>>>>> as first argument to .DEFERRED_INIT.
>>>>>>> 
>>>>>>> You mean something like:
>>>>>>> __s2_len_218 = .DEFERRED_INIT (__s2_len, 2);
>>>>>> 
>>>>>> No,
>>>>>> 
>>>>>> __s2_len_218 = .DEFERRED_INIT (__s2_len_217(D), 2);
>>>>>> 
>>>>>>> ?
>>>>>>> 
>>>>>>> 
>>>>>>> __s2_len_219 = 7;
>>>>>>> if (__s2_len_219 <= 3)
>>>>>>> goto <bb 4>; [INV]
>>>>>>> else
>>>>>>> goto <bb 9>; [INV]
>>>>>>> 
>>>>>>> <bb 4> :
>>>>>>> _1 = (long unsigned int) i_175;
>>>>>>> 
>>>>>>> 
>>>>>>> ****However, after “ccp”, in “t.i.032t.ccp1”, we have:
>>>>>>> 
>>>>>>> <bb 3> :
>>>>>>> __s1_len_217 = .DEFERRED_INIT (__s1_len_176, 2);
>>>>>>> __s2_len_218 = .DEFERRED_INIT (7, 2);
>>>>>>> _36 = (long unsigned int) i_175;
>>>>>>> _37 = _36 * 8;
>>>>>>> _38 = argv_220(D) + _37;
>>>>>>> 
>>>>>>> 
>>>>>>> Looks like that the optimization “ccp” replaced the first
>> argument
>>>> of the call .DEFERRED_INIT with the constant 7.
>>>>>>> This should be avoided.
>>>>>>> 
>>>>>>> (NOTE, this issue existed in the previous patches, however, only
>>>> exposed with this version since I added more verification
>>>>>>> code in tree-cfg.c to verify the call to .DEFERRED_INIT).
>>>>>>> 
>>>>>>> I am wondering what’s the best solution to this problem?
>>>>>>> 
>>>>>>> I think you have to trace where this "bogus" .DEFERRED_INIT comes
>>>> from
>>>>>>> originally.  Or alternatively, if this is unavoidable,
>>>>>>> 
>>>>>>> This is unavoidable, I believe.
>>>>>> 
>>>>>> I see but don't believe it yet ;)
>>>>>> 
>>>>>>> add "constant
>>>>>>> folding" of .DEFERRED_INIT so that defered init of an initialized
>>>>>>> object becomes the object itself, thus retain the previous -
>>>> eventually
>>>>>>> partial - initialization only.
>>>>>>> 
>>>>>>> If this additional .DEFERRED_INIT will be kept till RTL expansion
>>>> phase, then it will become a real initialization:
>>>>>>> 
>>>>>>> i.e.
>>>>>>> 
>>>>>>> s2_len = 0;    //.DEFERRED_INIT expanded
>>>>>>> s2_len = 4;    // the original initialization
>>>>>>> 
>>>>>>> Then the first initialization will be eliminated by current RTL
>>>> optimization easily, right?
>>>>>> 
>>>>>> Well, in your example above it's effectively elimiated by GIMPLE
>>>>>> optimization.  IIRC you're using the first argument of
>>>> .DEFERRED_INIT
>>>>>> for diagnostic purposes only, correct?
>>>>>> 
>>>>>> Richard.
>>>>>> 
>>>>>>> Qing
>>>>>>> 
>>>>>>> 
>>>>>>> Richard.
>>>>>>> 
>>>>>>> Can we add any attribute to the internal function argument to
>>>> prevent later optimizations that might applied on it?
>>>>>>> Or just update “ccp” phase to specially handle calls to
>>>> .DEFERRED_INIT? (Not sure whether there are other phases have the
>>>>>>> Same issue?)
>>>>>>> 
>>>>>>> Let me know if you have any suggestion.
>>>>>>> 
>>>>>>> Thanks a lot for your help.
>>>>>>> 
>>>>>>> Qing
>>>>>>> 
>>>>>>> --
>>>>>>> Richard Biener <rguenther@suse.de<mailto:rguenther@suse.de>>
>>>>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
>>>> Nuernberg,
>>>>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>>> --
>>>>>> Richard Biener <rguenther@suse.de>
>>>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
>>>> Nuernberg,
>>>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>>>> 
>>> 
> 

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

> On Jun 30, 2021, at 1:59 PM, Richard Biener <rguenther@suse.de> wrote:
> 
> On June 30, 2021 8:07:43 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
>> 
>> 
>>> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de>
>> wrote:
>>> 
>>> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski
>> <pinskia@gmail.com> wrote:
>>>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
>>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>> 
>>>>> I came up with a very simple testing case that can repeat the same
>>>> issue:
>>>>> 
>>>>> [qinzhao@localhost gcc]$ cat t.c
>>>>> extern void bar (int);
>>>>> void foo (int a)
>>>>> {
>>>>> int i;
>>>>> for (i = 0; i < a; i++) {
>>>>>   if (__extension__({int size2;
>>>>>       size2 = 4;
>>>>>       size2 > 5;}))
>>>>>   bar (a);
>>>>> }
>>>>> }
>>>> 
>>>> You should show the full dump,
>>>> What we have is the following:
>>>> 
>>>> 
>>>> 
>>>> size2_3 = PHI <size2_1(D), size2_13>
>>>> <bb 3> :
>>>> 
>>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
>>>> size2_13 = 4;
>>>> 
>>>> So CCP decides to propagate 4 into the PHI and then decides
>> size2_1(D)
>>>> is undefined so size2_3 is then considered 4 and propagates it into
>>>> the .DEFERRED_INIT.
>>> 
>>> Which means the DEFERED_INIT is inserted at the wrong place. 
>> 
>> Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
>> 
>> The variable “size2” is a block scope variable which is declared inside
>> the “if” condition:
> 
> But that's obviously not how it behaves
> During into SSA phase since we're inserting a PHI for it - and we're inserting it because of the use in the DEFERED_INIT call. I suppose you need to fiddle with the SSA rewrite and avoid treating the use as a use but only for the purpose of inserting PHIs...

Please see my other email on the new small testing case without -ftrivial-auto-var-init.  The same issue in SSA with that testing case even without -ftrivial-auto-var-init. 
It looks like an existing bug to me in SSA.

Let me know if I still miss anything 

Qing
> 
> You might be able to construct a testcase which has a use before the real init where then the optimistic CCP propagation will defeat the DEFERED_INIT otherwise. 
> 
> I'd need to play with the actual patch to find a good solution to this problem. 
> 
> Richard.
> 

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Andrew Pinski]

On Wed, Jun 30, 2021 at 12:14 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>
>
>
> > On Jun 30, 2021, at 1:59 PM, Richard Biener <rguenther@suse.de> wrote:
> >
> > On June 30, 2021 8:07:43 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
> >>
> >>
> >>> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de>
> >> wrote:
> >>>
> >>> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski
> >> <pinskia@gmail.com> wrote:
> >>>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
> >>>> <gcc-patches@gcc.gnu.org> wrote:
> >>>>>
> >>>>> I came up with a very simple testing case that can repeat the same
> >>>> issue:
> >>>>>
> >>>>> [qinzhao@localhost gcc]$ cat t.c
> >>>>> extern void bar (int);
> >>>>> void foo (int a)
> >>>>> {
> >>>>> int i;
> >>>>> for (i = 0; i < a; i++) {
> >>>>>   if (__extension__({int size2;
> >>>>>       size2 = 4;
> >>>>>       size2 > 5;}))
> >>>>>   bar (a);
> >>>>> }
> >>>>> }
> >>>>
> >>>> You should show the full dump,
> >>>> What we have is the following:
> >>>>
> >>>>
> >>>>
> >>>> size2_3 = PHI <size2_1(D), size2_13>
> >>>> <bb 3> :
> >>>>
> >>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
> >>>> size2_13 = 4;
> >>>>
> >>>> So CCP decides to propagate 4 into the PHI and then decides
> >> size2_1(D)
> >>>> is undefined so size2_3 is then considered 4 and propagates it into
> >>>> the .DEFERRED_INIT.
> >>>
> >>> Which means the DEFERED_INIT is inserted at the wrong place.
> >>
> >> Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
> >>
> >> The variable “size2” is a block scope variable which is declared inside
> >> the “if” condition:
> >
> > But that's obviously not how it behaves
> > During into SSA phase since we're inserting a PHI for it - and we're inserting it because of the use in the DEFERED_INIT call. I suppose you need to fiddle with the SSA rewrite and avoid treating the use as a use but only for the purpose of inserting PHIs...
>
> Please see my other email on the new small testing case without -ftrivial-auto-var-init.  The same issue in SSA with that testing case even without -ftrivial-auto-var-init.
> It looks like an existing bug to me in SSA.
>
> Let me know if I still miss anything

Yes you missed it is unspecified what the value if the auto variable
is used uninitialized. Isn't that the point of what you are trying to
fix in the first place?
So ccp takes PHI<a_1(D), 4> and says since a_1(D) is uninitialized,
the value is 4.

Thanks,
Andrew

>
> Qing
> >
> > You might be able to construct a testcase which has a use before the real init where then the optimistic CCP propagation will defeat the DEFERED_INIT otherwise.
> >
> > I'd need to play with the actual patch to find a good solution to this problem.
> >
> > Richard.
> >
>

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

Hi, Andrew:

> On Jun 30, 2021, at 2:20 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>>>> 
>>>> 
>>>> Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
>>>> 
>>>> The variable “size2” is a block scope variable which is declared inside
>>>> the “if” condition:
>>> 
>>> But that's obviously not how it behaves
>>> During into SSA phase since we're inserting a PHI for it - and we're inserting it because of the use in the DEFERED_INIT call. I suppose you need to fiddle with the SSA rewrite and avoid treating the use as a use but only for the purpose of inserting PHIs...
>> 
>> Please see my other email on the new small testing case without -ftrivial-auto-var-init.  The same issue in SSA with that testing case even without -ftrivial-auto-var-init.
>> It looks like an existing bug to me in SSA.
>> 
>> Let me know if I still miss anything
> 
> Yes you missed it is unspecified what the value if the auto variable
> is used uninitialized. Isn't that the point of what you are trying to
> fix in the first place?
> So ccp takes PHI<a_1(D), 4> and says since a_1(D) is uninitialized,
> the value is 4.

I don’t think CCP did anything wrong.
I suspect that SSA did something wrong when constructing PHI node for the block-scope auto variables.

For the following example:

  1 extern void bar (int);
  2 extern int ART_INIT(int, int);
  3 void foo (int a)
  4 {
  5   int i;
  6   for (i = 0; i < a; i++) {
  7     if (__extension__({int size2;
  8         size2 = ART_INIT (size2, 2);
  9         size2 = 4;
 10         size2 > 5;}))
 11     bar (a);
 12   }
 13 }
~                                                                               

Is it legal to propagate constant 4 at line 9 to the first argument of call to ART_INIT at line 8?
Given size2 is a block-scope auto-variable whose scope is from line 7 to line 10. The value of size2 should not be carried over through
loop iterations from my understanding. 

Qing

> 
> Thanks,
> Andrew
> 
>> 
>> Qing
>>> 
>>> You might be able to construct a testcase which has a use before the real init where then the optimistic CCP propagation will defeat the DEFERED_INIT otherwise.
>>> 
>>> I'd need to play with the actual patch to find a good solution to this problem.
>>> 
>>> Richard.
>>> 
>> 

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Richard Biener]

On Wed, Jun 30, 2021 at 9:15 PM Qing Zhao via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> > On Jun 30, 2021, at 1:59 PM, Richard Biener <rguenther@suse.de> wrote:
> >
> > On June 30, 2021 8:07:43 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
> >>
> >>
> >>> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de>
> >> wrote:
> >>>
> >>> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski
> >> <pinskia@gmail.com> wrote:
> >>>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
> >>>> <gcc-patches@gcc.gnu.org> wrote:
> >>>>>
> >>>>> I came up with a very simple testing case that can repeat the same
> >>>> issue:
> >>>>>
> >>>>> [qinzhao@localhost gcc]$ cat t.c
> >>>>> extern void bar (int);
> >>>>> void foo (int a)
> >>>>> {
> >>>>> int i;
> >>>>> for (i = 0; i < a; i++) {
> >>>>>   if (__extension__({int size2;
> >>>>>       size2 = 4;
> >>>>>       size2 > 5;}))
> >>>>>   bar (a);
> >>>>> }
> >>>>> }
> >>>>
> >>>> You should show the full dump,
> >>>> What we have is the following:
> >>>>
> >>>>
> >>>>
> >>>> size2_3 = PHI <size2_1(D), size2_13>
> >>>> <bb 3> :
> >>>>
> >>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
> >>>> size2_13 = 4;
> >>>>
> >>>> So CCP decides to propagate 4 into the PHI and then decides
> >> size2_1(D)
> >>>> is undefined so size2_3 is then considered 4 and propagates it into
> >>>> the .DEFERRED_INIT.
> >>>
> >>> Which means the DEFERED_INIT is inserted at the wrong place.
> >>
> >> Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
> >>
> >> The variable “size2” is a block scope variable which is declared inside
> >> the “if” condition:
> >
> > But that's obviously not how it behaves
> > During into SSA phase since we're inserting a PHI for it - and we're inserting it because of the use in the DEFERED_INIT call. I suppose you need to fiddle with the SSA rewrite and avoid treating the use as a use but only for the purpose of inserting PHIs...
>
> Please see my other email on the new small testing case without -ftrivial-auto-var-init.  The same issue in SSA with that testing case even without -ftrivial-auto-var-init.
> It looks like an existing bug to me in SSA.

It's not a bug in SSA, it's at most a missed optimization there.  But
with -ftrivial-auto-var-init it
becomes a correctness issue.  I think the idea avoiding the USE of the
variable in .DEFERRED_INIT
and instead passing the init size is a good one and should avoid this
case (hopefully).

Richard.

> Let me know if I still miss anything
>
> Qing
> >
> > You might be able to construct a testcase which has a use before the real init where then the optimistic CCP propagation will defeat the DEFERED_INIT otherwise.
> >
> > I'd need to play with the actual patch to find a good solution to this problem.
> >
> > Richard.
> >
>

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

> On Jul 1, 2021, at 1:48 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Wed, Jun 30, 2021 at 9:15 PM Qing Zhao via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> 
>> 
>> 
>>> On Jun 30, 2021, at 1:59 PM, Richard Biener <rguenther@suse.de> wrote:
>>> 
>>> On June 30, 2021 8:07:43 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
>>>> 
>>>> 
>>>>> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de>
>>>> wrote:
>>>>> 
>>>>> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski
>>>> <pinskia@gmail.com> wrote:
>>>>>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
>>>>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>>>> 
>>>>>>> I came up with a very simple testing case that can repeat the same
>>>>>> issue:
>>>>>>> 
>>>>>>> [qinzhao@localhost gcc]$ cat t.c
>>>>>>> extern void bar (int);
>>>>>>> void foo (int a)
>>>>>>> {
>>>>>>> int i;
>>>>>>> for (i = 0; i < a; i++) {
>>>>>>>  if (__extension__({int size2;
>>>>>>>      size2 = 4;
>>>>>>>      size2 > 5;}))
>>>>>>>  bar (a);
>>>>>>> }
>>>>>>> }
>>>>>> 
>>>>>> You should show the full dump,
>>>>>> What we have is the following:
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> size2_3 = PHI <size2_1(D), size2_13>
>>>>>> <bb 3> :
>>>>>> 
>>>>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
>>>>>> size2_13 = 4;
>>>>>> 
>>>>>> So CCP decides to propagate 4 into the PHI and then decides
>>>> size2_1(D)
>>>>>> is undefined so size2_3 is then considered 4 and propagates it into
>>>>>> the .DEFERRED_INIT.
>>>>> 
>>>>> Which means the DEFERED_INIT is inserted at the wrong place.
>>>> 
>>>> Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
>>>> 
>>>> The variable “size2” is a block scope variable which is declared inside
>>>> the “if” condition:
>>> 
>>> But that's obviously not how it behaves
>>> During into SSA phase since we're inserting a PHI for it - and we're inserting it because of the use in the DEFERED_INIT call. I suppose you need to fiddle with the SSA rewrite and avoid treating the use as a use but only for the purpose of inserting PHIs...
>> 
>> Please see my other email on the new small testing case without -ftrivial-auto-var-init.  The same issue in SSA with that testing case even without -ftrivial-auto-var-init.
>> It looks like an existing bug to me in SSA.
> 
> It's not a bug in SSA, it's at most a missed optimization there.

I still think that SSA cannot handle block-scoped variable correctly in this case, it should not move the variable out side of the block scope. -:)

>  But
> with -ftrivial-auto-var-init it
> becomes a correctness issue.  I think the idea avoiding the USE of the
> variable in .DEFERRED_INIT
> and instead passing the init size is a good one and should avoid this
> case (hopefully).


Agreed, I have made such change yesterday, and this work around the current issue.

temp = .DEFERRED_INIT (temp/WITH_SIZE_EXPR(temp), init_type)

To:

temp = .DEFERRED_INIT (SIZE_of_temp, init_type)

Thanks.

Qing
> 
> Richard.
> 
>> Let me know if I still miss anything
>> 
>> Qing
>>> 
>>> You might be able to construct a testcase which has a use before the real init where then the optimistic CCP propagation will defeat the DEFERED_INIT otherwise.
>>> 
>>> I'd need to play with the actual patch to find a good solution to this problem.
>>> 
>>> Richard.
>>> 
>> 

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Richard Biener]

On Thu, Jul 1, 2021 at 3:45 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>
>
>
> > On Jul 1, 2021, at 1:48 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> >
> > On Wed, Jun 30, 2021 at 9:15 PM Qing Zhao via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >>
> >>
> >>> On Jun 30, 2021, at 1:59 PM, Richard Biener <rguenther@suse.de> wrote:
> >>>
> >>> On June 30, 2021 8:07:43 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
> >>>>
> >>>>
> >>>>> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de>
> >>>> wrote:
> >>>>>
> >>>>> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski
> >>>> <pinskia@gmail.com> wrote:
> >>>>>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
> >>>>>> <gcc-patches@gcc.gnu.org> wrote:
> >>>>>>>
> >>>>>>> I came up with a very simple testing case that can repeat the same
> >>>>>> issue:
> >>>>>>>
> >>>>>>> [qinzhao@localhost gcc]$ cat t.c
> >>>>>>> extern void bar (int);
> >>>>>>> void foo (int a)
> >>>>>>> {
> >>>>>>> int i;
> >>>>>>> for (i = 0; i < a; i++) {
> >>>>>>>  if (__extension__({int size2;
> >>>>>>>      size2 = 4;
> >>>>>>>      size2 > 5;}))
> >>>>>>>  bar (a);
> >>>>>>> }
> >>>>>>> }
> >>>>>>
> >>>>>> You should show the full dump,
> >>>>>> What we have is the following:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> size2_3 = PHI <size2_1(D), size2_13>
> >>>>>> <bb 3> :
> >>>>>>
> >>>>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
> >>>>>> size2_13 = 4;
> >>>>>>
> >>>>>> So CCP decides to propagate 4 into the PHI and then decides
> >>>> size2_1(D)
> >>>>>> is undefined so size2_3 is then considered 4 and propagates it into
> >>>>>> the .DEFERRED_INIT.
> >>>>>
> >>>>> Which means the DEFERED_INIT is inserted at the wrong place.
> >>>>
> >>>> Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
> >>>>
> >>>> The variable “size2” is a block scope variable which is declared inside
> >>>> the “if” condition:
> >>>
> >>> But that's obviously not how it behaves
> >>> During into SSA phase since we're inserting a PHI for it - and we're inserting it because of the use in the DEFERED_INIT call. I suppose you need to fiddle with the SSA rewrite and avoid treating the use as a use but only for the purpose of inserting PHIs...
> >>
> >> Please see my other email on the new small testing case without -ftrivial-auto-var-init.  The same issue in SSA with that testing case even without -ftrivial-auto-var-init.
> >> It looks like an existing bug to me in SSA.
> >
> > It's not a bug in SSA, it's at most a missed optimization there.
>
> I still think that SSA cannot handle block-scoped variable correctly in this case, it should not move the variable out side of the block scope. -:)

Well, the SSA rewriter simply does not "understand" scopes and thus
keeps registers live across
the scope boundaries which results in unnecessary PHI nodes in this
particular case.  That's of
course because "scopes" no longer exist at this point in the IL, they
are at most approximated
by CLOBBERs we put at the end of scopes, but we don't do that for everything.

For

void foo ()
{
lab:
    {
      int i;
      i += 1;
    }
  goto lab;
}

we get

  <bb 2> :
  # i_1 = PHI <i_2(D)(0), i_3(2)>
lab:
  i_3 = i_1 + 1;
  // predicted unlikely by goto predictor.
  goto <bb 2>; [INV]

but

  <bb 2> :
lab:
  i_3 = i_2(D) + 1;
  // predicted unlikely by goto predictor.
  goto <bb 2>; [INV]

would have been correct as  well.

Richard.

> >  But
> > with -ftrivial-auto-var-init it
> > becomes a correctness issue.  I think the idea avoiding the USE of the
> > variable in .DEFERRED_INIT
> > and instead passing the init size is a good one and should avoid this
> > case (hopefully).
>
>
> Agreed, I have made such change yesterday, and this work around the current issue.
>
> temp = .DEFERRED_INIT (temp/WITH_SIZE_EXPR(temp), init_type)
>
> To:
>
> temp = .DEFERRED_INIT (SIZE_of_temp, init_type)
>
> Thanks.
>
> Qing
> >
> > Richard.
> >
> >> Let me know if I still miss anything
> >>
> >> Qing
> >>>
> >>> You might be able to construct a testcase which has a use before the real init where then the optimistic CCP propagation will defeat the DEFERED_INIT otherwise.
> >>>
> >>> I'd need to play with the actual patch to find a good solution to this problem.
> >>>
> >>> Richard.
> >>>
> >>
>

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Richard Sandiford]

Qing Zhao <qing.zhao@oracle.com> writes:
>> On Jul 1, 2021, at 1:48 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>> 
>> On Wed, Jun 30, 2021 at 9:15 PM Qing Zhao via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>> 
>>> 
>>> 
>>>> On Jun 30, 2021, at 1:59 PM, Richard Biener <rguenther@suse.de> wrote:
>>>> 
>>>> On June 30, 2021 8:07:43 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
>>>>> 
>>>>> 
>>>>>> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de>
>>>>> wrote:
>>>>>> 
>>>>>> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski
>>>>> <pinskia@gmail.com> wrote:
>>>>>>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
>>>>>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>>>>> 
>>>>>>>> I came up with a very simple testing case that can repeat the same
>>>>>>> issue:
>>>>>>>> 
>>>>>>>> [qinzhao@localhost gcc]$ cat t.c
>>>>>>>> extern void bar (int);
>>>>>>>> void foo (int a)
>>>>>>>> {
>>>>>>>> int i;
>>>>>>>> for (i = 0; i < a; i++) {
>>>>>>>>  if (__extension__({int size2;
>>>>>>>>      size2 = 4;
>>>>>>>>      size2 > 5;}))
>>>>>>>>  bar (a);
>>>>>>>> }
>>>>>>>> }
>>>>>>> 
>>>>>>> You should show the full dump,
>>>>>>> What we have is the following:
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> size2_3 = PHI <size2_1(D), size2_13>
>>>>>>> <bb 3> :
>>>>>>> 
>>>>>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
>>>>>>> size2_13 = 4;
>>>>>>> 
>>>>>>> So CCP decides to propagate 4 into the PHI and then decides
>>>>> size2_1(D)
>>>>>>> is undefined so size2_3 is then considered 4 and propagates it into
>>>>>>> the .DEFERRED_INIT.
>>>>>> 
>>>>>> Which means the DEFERED_INIT is inserted at the wrong place.
>>>>> 
>>>>> Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
>>>>> 
>>>>> The variable “size2” is a block scope variable which is declared inside
>>>>> the “if” condition:
>>>> 
>>>> But that's obviously not how it behaves
>>>> During into SSA phase since we're inserting a PHI for it - and we're inserting it because of the use in the DEFERED_INIT call. I suppose you need to fiddle with the SSA rewrite and avoid treating the use as a use but only for the purpose of inserting PHIs...
>>> 
>>> Please see my other email on the new small testing case without -ftrivial-auto-var-init.  The same issue in SSA with that testing case even without -ftrivial-auto-var-init.
>>> It looks like an existing bug to me in SSA.
>> 
>> It's not a bug in SSA, it's at most a missed optimization there.
>
> I still think that SSA cannot handle block-scoped variable correctly in this case, it should not move the variable out side of the block scope. -:)
>
>>  But
>> with -ftrivial-auto-var-init it
>> becomes a correctness issue.

I might have lost track of what “it” is here.  Do you mean the progation,
or the fact that we have a PHI in the first place?

For:

unsigned int
f (int n)
{
  unsigned int res = 0;
  for (int i = 0; i < n; i += 1)
    {
      unsigned int foo;
      foo += 1;
      res = foo;
    }
  return res;
}

we generate:

  unsigned int foo;
  int i;
  unsigned int res;
  unsigned int _8;

  <bb 2> :
  res_4 = 0;
  i_5 = 0;
  goto <bb 4>; [INV]

  <bb 3> :
  foo_10 = foo_3 + 1;
  res_11 = foo_10;
  i_12 = i_2 + 1;

  <bb 4> :
  # res_1 = PHI <res_4(2), res_11(3)>
  # i_2 = PHI <i_5(2), i_12(3)>
  # foo_3 = PHI <foo_6(D)(2), foo_10(3)>
  if (i_2 < n_7(D))
    goto <bb 3>; [INV]
  else
    goto <bb 5>; [INV]

  <bb 5> :
  _8 = res_1;
  return _8;

IMO the foo_3 PHI makes no sense.  foo doesn't live beyond its block,
so there should be no loop-carried dependency here.

So yeah, if “it” meant that the fact that variables live too long,
then I agree that becomes a correctness issue with this feature,
rather than just an odd quirk.  Could we solve it by inserting
a clobber at the end of the variable's scope, or something like that?

>> I think the idea avoiding the USE of the variable in .DEFERRED_INIT
>> and instead passing the init size is a good one and should avoid this
>> case (hopefully).
>
>
> Agreed, I have made such change yesterday, and this work around the current issue.
>
> temp = .DEFERRED_INIT (temp/WITH_SIZE_EXPR(temp), init_type)
>
> To:
>
> temp = .DEFERRED_INIT (SIZE_of_temp, init_type)

I think we're going round in circles here though.  The point of having
the undefined input was so that we would continue to warn about undefined
inputs.  The above issue doesn't seem like a good justification for
dropping that.

Thanks,
Richard

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Michael Matz]

Hello,

I haven't followed this thread too closely, in particular I haven't seen 
why the current form of the .DEFERRED_INIT call was chosen or suggested, 
but it triggered my "well, that's obviously wrong" gut feeling; so sorry 
for stating something which might be obvious to thread participants here.  
Anyway:

On Thu, 1 Jul 2021, Richard Sandiford via Gcc-patches wrote:

> >> It's not a bug in SSA, it's at most a missed optimization there.
> >
> > I still think that SSA cannot handle block-scoped variable correctly 
> > in this case, it should not move the variable out side of the block 
> > scope. -:)
> >
> >>  But with -ftrivial-auto-var-init it becomes a correctness issue.
> 
> I might have lost track of what “it” is here.  Do you mean the 
> progation, or the fact that we have a PHI in the first place?
> 
> For:
> 
> unsigned int
> f (int n)
> {
>   unsigned int res = 0;
>   for (int i = 0; i < n; i += 1)
>     {
>       unsigned int foo;
>       foo += 1;

Note that here foo is used uninitialized.  That is the thing from which 
everything else follows.  Garbage in, garbage out.  It makes not too much 
sense to argue that the generated PHI node on this loop (generated because 
of the exposed-upward use of foo) is wrong, or right, or should better be 
something else.  The input program was broken, so anything makes sense.

That is the same with Qings shorter testcase:

  6   for (i = 0; i < a; i++) {
  7     if (__extension__({int size2;
  8         size2 = ART_INIT (size2, 2);

Uninitialized use of size2 right there.  And it's the same for the use of 
.DEFERRED_INIT as the patch does:

{
  int size2;
  size2 = .DEFERRED_INIT (size2, 2);
  size2 = 4;
  _1 = size2 > 5;
  D.2240 = (int) _1;
}

Argument of the pseudo-function-call to .DEFERRED_INIT uninitialized -> 
boom.

You can't solve any of this by fiddling with how SSA rewriting behaves at 
a large scale.  You need to change this and only this specific 
interpretation of a use.  Or preferrably not generate it to start with.

> IMO the foo_3 PHI makes no sense.  foo doesn't live beyond its block,
> so there should be no loop-carried dependency here.
> 
> So yeah, if “it” meant that the fact that variables live too long,
> then I agree that becomes a correctness issue with this feature,
> rather than just an odd quirk.  Could we solve it by inserting
> a clobber at the end of the variable's scope, or something like that?

It would possibly make GCC not generate a PHI on this broken input, yes.  
But why would that be an improvement?

> > Agreed, I have made such change yesterday, and this work around the 
> > current issue.
> >
> > temp = .DEFERRED_INIT (temp/WITH_SIZE_EXPR(temp), init_type)
> >
> > To:
> >
> > temp = .DEFERRED_INIT (SIZE_of_temp, init_type)
> 
> I think we're going round in circles here though.  The point of having
> the undefined input was so that we would continue to warn about undefined
> inputs.  The above issue doesn't seem like a good justification for
> dropping that.

If you want to rely on the undefined use for error reporting then you must 
only generate an undefined use when there was one before, you can't just 
insert new undefined uses.  I don't see how it could be otherwise, as soon 
as you introduce new undefined uses you can and will run into GCC making 
use of the undefinedness, not just this particular issue with lifetime and 
PHI nodes which might be "solved" by clobbers.

I think it's a chicken egg problem: you can't add undefined uses, for 
which you need to know if there was one, but the facility is supposed to 
help detecting if there is one to start with.


Ciao,
Michael.

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

> On Jul 1, 2021, at 9:04 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Thu, Jul 1, 2021 at 3:45 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>> 
>> 
>> 
>>> On Jul 1, 2021, at 1:48 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>> 
>>> On Wed, Jun 30, 2021 at 9:15 PM Qing Zhao via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On Jun 30, 2021, at 1:59 PM, Richard Biener <rguenther@suse.de> wrote:
>>>>> 
>>>>> On June 30, 2021 8:07:43 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
>>>>>> 
>>>>>> 
>>>>>>> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de>
>>>>>> wrote:
>>>>>>> 
>>>>>>> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski
>>>>>> <pinskia@gmail.com> wrote:
>>>>>>>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
>>>>>>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>>>>>> 
>>>>>>>>> I came up with a very simple testing case that can repeat the same
>>>>>>>> issue:
>>>>>>>>> 
>>>>>>>>> [qinzhao@localhost gcc]$ cat t.c
>>>>>>>>> extern void bar (int);
>>>>>>>>> void foo (int a)
>>>>>>>>> {
>>>>>>>>> int i;
>>>>>>>>> for (i = 0; i < a; i++) {
>>>>>>>>> if (__extension__({int size2;
>>>>>>>>>     size2 = 4;
>>>>>>>>>     size2 > 5;}))
>>>>>>>>> bar (a);
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>> 
>>>>>>>> You should show the full dump,
>>>>>>>> What we have is the following:
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> size2_3 = PHI <size2_1(D), size2_13>
>>>>>>>> <bb 3> :
>>>>>>>> 
>>>>>>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
>>>>>>>> size2_13 = 4;
>>>>>>>> 
>>>>>>>> So CCP decides to propagate 4 into the PHI and then decides
>>>>>> size2_1(D)
>>>>>>>> is undefined so size2_3 is then considered 4 and propagates it into
>>>>>>>> the .DEFERRED_INIT.
>>>>>>> 
>>>>>>> Which means the DEFERED_INIT is inserted at the wrong place.
>>>>>> 
>>>>>> Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
>>>>>> 
>>>>>> The variable “size2” is a block scope variable which is declared inside
>>>>>> the “if” condition:
>>>>> 
>>>>> But that's obviously not how it behaves
>>>>> During into SSA phase since we're inserting a PHI for it - and we're inserting it because of the use in the DEFERED_INIT call. I suppose you need to fiddle with the SSA rewrite and avoid treating the use as a use but only for the purpose of inserting PHIs...
>>>> 
>>>> Please see my other email on the new small testing case without -ftrivial-auto-var-init.  The same issue in SSA with that testing case even without -ftrivial-auto-var-init.
>>>> It looks like an existing bug to me in SSA.
>>> 
>>> It's not a bug in SSA, it's at most a missed optimization there.
>> 
>> I still think that SSA cannot handle block-scoped variable correctly in this case, it should not move the variable out side of the block scope. -:)
> 
> Well, the SSA rewriter simply does not "understand" scopes and thus
> keeps registers live across
> the scope boundaries which results in unnecessary PHI nodes in this
> particular case.  That's of
> course because "scopes" no longer exist at this point in the IL, they
> are at most approximated
> by CLOBBERs we put at the end of scopes, but we don't do that for everything.

Then, this looks like a bug to me.
If the scope info is emulated by CLOBBERs in this stage, then compiler should add CLOBBERs at proper places to
keep the scope info. 
Moving a variable out of its live scope seems not a correct transformation.

> 
> For
> 
> void foo ()
> {
> lab:
>    {
>      int i;
>      i += 1;
>    }
>  goto lab;
> }
> 
> we get
> 
>  <bb 2> :
>  # i_1 = PHI <i_2(D)(0), i_3(2)>

Here, the variable “I” had been moved out of its live scope already. 
Seems not correct.

Qing
> lab:
>  i_3 = i_1 + 1;
>  // predicted unlikely by goto predictor.
>  goto <bb 2>; [INV]
> 
> but
> 
>  <bb 2> :
> lab:
>  i_3 = i_2(D) + 1;
>  // predicted unlikely by goto predictor.
>  goto <bb 2>; [INV]
> 
> would have been correct as  well.
> 
> Richard.
> 
>>> But
>>> with -ftrivial-auto-var-init it
>>> becomes a correctness issue.  I think the idea avoiding the USE of the
>>> variable in .DEFERRED_INIT
>>> and instead passing the init size is a good one and should avoid this
>>> case (hopefully).
>> 
>> 
>> Agreed, I have made such change yesterday, and this work around the current issue.
>> 
>> temp = .DEFERRED_INIT (temp/WITH_SIZE_EXPR(temp), init_type)
>> 
>> To:
>> 
>> temp = .DEFERRED_INIT (SIZE_of_temp, init_type)
>> 
>> Thanks.
>> 
>> Qing
>>> 
>>> Richard.
>>> 
>>>> Let me know if I still miss anything
>>>> 
>>>> Qing
>>>>> 
>>>>> You might be able to construct a testcase which has a use before the real init where then the optimistic CCP propagation will defeat the DEFERED_INIT otherwise.
>>>>> 
>>>>> I'd need to play with the actual patch to find a good solution to this problem.
>>>>> 
>>>>> Richard.
>>>>> 
>>>> 
>> 

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

> On Jul 1, 2021, at 9:09 AM, Richard Sandiford <richard.sandiford@arm.com> wrote:
> 
> Qing Zhao <qing.zhao@oracle.com> writes:
>>> On Jul 1, 2021, at 1:48 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>> 
>>> On Wed, Jun 30, 2021 at 9:15 PM Qing Zhao via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On Jun 30, 2021, at 1:59 PM, Richard Biener <rguenther@suse.de> wrote:
>>>>> 
>>>>> On June 30, 2021 8:07:43 PM GMT+02:00, Qing Zhao <qing.zhao@oracle.com> wrote:
>>>>>> 
>>>>>> 
>>>>>>> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguenther@suse.de>
>>>>>> wrote:
>>>>>>> 
>>>>>>> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski
>>>>>> <pinskia@gmail.com> wrote:
>>>>>>>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
>>>>>>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>>>>>> 
>>>>>>>>> I came up with a very simple testing case that can repeat the same
>>>>>>>> issue:
>>>>>>>>> 
>>>>>>>>> [qinzhao@localhost gcc]$ cat t.c
>>>>>>>>> extern void bar (int);
>>>>>>>>> void foo (int a)
>>>>>>>>> {
>>>>>>>>> int i;
>>>>>>>>> for (i = 0; i < a; i++) {
>>>>>>>>> if (__extension__({int size2;
>>>>>>>>>     size2 = 4;
>>>>>>>>>     size2 > 5;}))
>>>>>>>>> bar (a);
>>>>>>>>> }
>>>>>>>>> }
>>>>>>>> 
>>>>>>>> You should show the full dump,
>>>>>>>> What we have is the following:
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> size2_3 = PHI <size2_1(D), size2_13>
>>>>>>>> <bb 3> :
>>>>>>>> 
>>>>>>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
>>>>>>>> size2_13 = 4;
>>>>>>>> 
>>>>>>>> So CCP decides to propagate 4 into the PHI and then decides
>>>>>> size2_1(D)
>>>>>>>> is undefined so size2_3 is then considered 4 and propagates it into
>>>>>>>> the .DEFERRED_INIT.
>>>>>>> 
>>>>>>> Which means the DEFERED_INIT is inserted at the wrong place.
>>>>>> 
>>>>>> Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
>>>>>> 
>>>>>> The variable “size2” is a block scope variable which is declared inside
>>>>>> the “if” condition:
>>>>> 
>>>>> But that's obviously not how it behaves
>>>>> During into SSA phase since we're inserting a PHI for it - and we're inserting it because of the use in the DEFERED_INIT call. I suppose you need to fiddle with the SSA rewrite and avoid treating the use as a use but only for the purpose of inserting PHIs...
>>>> 
>>>> Please see my other email on the new small testing case without -ftrivial-auto-var-init.  The same issue in SSA with that testing case even without -ftrivial-auto-var-init.
>>>> It looks like an existing bug to me in SSA.
>>> 
>>> It's not a bug in SSA, it's at most a missed optimization there.
>> 
>> I still think that SSA cannot handle block-scoped variable correctly in this case, it should not move the variable out side of the block scope. -:)
>> 
>>> But
>>> with -ftrivial-auto-var-init it
>>> becomes a correctness issue.
> 
> I might have lost track of what “it” is here.  Do you mean the progation,
> or the fact that we have a PHI in the first place?

“PHI” out of the live scope of the corresponding variable. 

> 
> For:
> 
> unsigned int
> f (int n)
> {
>  unsigned int res = 0;
>  for (int i = 0; i < n; i += 1)
>    {
>      unsigned int foo;
>      foo += 1;
>      res = foo;
>    }
>  return res;
> }
> 
> we generate:
> 
>  unsigned int foo;
>  int i;
>  unsigned int res;
>  unsigned int _8;
> 
>  <bb 2> :
>  res_4 = 0;
>  i_5 = 0;
>  goto <bb 4>; [INV]
> 
>  <bb 3> :
>  foo_10 = foo_3 + 1;
>  res_11 = foo_10;
>  i_12 = i_2 + 1;
> 
>  <bb 4> :
>  # res_1 = PHI <res_4(2), res_11(3)>
>  # i_2 = PHI <i_5(2), i_12(3)>
>  # foo_3 = PHI <foo_6(D)(2), foo_10(3)>
>  if (i_2 < n_7(D))
>    goto <bb 3>; [INV]
>  else
>    goto <bb 5>; [INV]
> 
>  <bb 5> :
>  _8 = res_1;
>  return _8;
> 
> IMO the foo_3 PHI makes no sense.  foo doesn't live beyond its block,
> so there should be no loop-carried dependency here.
> 
> So yeah, if “it” meant that the fact that variables live too long,
> then I agree that becomes a correctness issue with this feature,
> rather than just an odd quirk.

Yes, I think so.

>  Could we solve it by inserting
> a clobber at the end of the variable's scope, or something like that?
Yes, I think that will be a good solution.
> 
>>> I think the idea avoiding the USE of the variable in .DEFERRED_INIT
>>> and instead passing the init size is a good one and should avoid this
>>> case (hopefully).
>> 
>> 
>> Agreed, I have made such change yesterday, and this work around the current issue.
>> 
>> temp = .DEFERRED_INIT (temp/WITH_SIZE_EXPR(temp), init_type)
>> 
>> To:
>> 
>> temp = .DEFERRED_INIT (SIZE_of_temp, init_type)
> 
> I think we're going round in circles here though.  The point of having
> the undefined input was so that we would continue to warn about undefined
> inputs.  The above issue doesn't seem like a good justification for
> dropping that.

Actually, in the current patch, the .DEFERRED_INIT calls are handled specially for uninitialized warnings
Analysis, this call itself is treated as the undefined input. 
Therefore, even after I made the above change, all the uninitialized warnings still are kept well.

So, I think that this change should be fine for keeping the uninitialized warnings.

Qing


> 
> Thanks,
> Richard
> 
> 

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Richard Sandiford]

Michael Matz <matz@suse.de> writes:
> Hello,
>
> I haven't followed this thread too closely, in particular I haven't seen 
> why the current form of the .DEFERRED_INIT call was chosen or suggested, 
> but it triggered my "well, that's obviously wrong" gut feeling; so sorry 
> for stating something which might be obvious to thread participants here.  

Well, it does feel like this is pressing the reset button on a thread
whose message count is already in the high double figures.  There's a
risk that restarting the discussion now is going to repeat a lot of the
same ground, probably at similar length.  (But I realise that the sheer
length of the thread is probably also the main reason for not having
followed it closely. :-))

To recap, the purpose of the option is to do two things simultaneously:

(1) make the object code behave “as if” automatic variables were
    initialised with a fill pattern, before any explicit initialisation

(2) continue to treat uses of uninitialised automatic variables as
    (semantically) undefined behaviour

When this was discussed on the clang list, (2) was a key requirement
and was instrumental in getting the feature accepted.  The option is
specificially *not* trying to change language semantics or create
a fork in the language.

The option is both a security and a debugging feature.  As a security
feature, it would be legitimate to optimise:

   int foo;
   if (flag)
     foo = 1;
   x = foo;

into:

   x = 1;

since the behaviour is at least deterministic.  But from a debugging QoI
perspective, this should not happen.  We should generate the same code
as for:

   int foo = <fill-pattern>;
   if (flag)
     foo = 1;
   x = foo;

However, because of (2), we should still generate a -Wmaybe-uninitialized
warning for the “x = foo” assignment.

This is not a combination we support at the moment, so something needs
to give.  The question of course is what.

> Anyway:
>
> On Thu, 1 Jul 2021, Richard Sandiford via Gcc-patches wrote:
>
>> >> It's not a bug in SSA, it's at most a missed optimization there.
>> >
>> > I still think that SSA cannot handle block-scoped variable correctly 
>> > in this case, it should not move the variable out side of the block 
>> > scope. -:)
>> >
>> >>  But with -ftrivial-auto-var-init it becomes a correctness issue.
>> 
>> I might have lost track of what “it” is here.  Do you mean the 
>> progation, or the fact that we have a PHI in the first place?
>> 
>> For:
>> 
>> unsigned int
>> f (int n)
>> {
>>   unsigned int res = 0;
>>   for (int i = 0; i < n; i += 1)
>>     {
>>       unsigned int foo;
>>       foo += 1;
>
> Note that here foo is used uninitialized.  That is the thing from which 
> everything else follows.

Right, that was the point.

> Garbage in, garbage out.  It makes not too much sense to argue that
> the generated PHI node on this loop (generated because of the
> exposed-upward use of foo) is wrong, or right, or should better be
> something else.  The input program was broken, so anything makes sense.

Yeah, without the new option it's GIGO.  But I think it's still possible
to rank different forms of garbage output in terms of how self-consistent
they are.

IMO it makes no sense to say that “foo” is upwards-exposed to somewhere
where “foo” doesn't exist.  Using “foo_N(D)” doesn't have that problem,
so I think it's possible to say that it's “better” garbage output. :-)

And the point is that with the new option, this is no longer garbage
input as far as SSA is concerned: carrying the old value of “foo”
across iterations would not be implementing the option correctly.
How SSA handles this becomes a correctness issue.

> That is the same with Qings shorter testcase:
>
>   6   for (i = 0; i < a; i++) {
>   7     if (__extension__({int size2;
>   8         size2 = ART_INIT (size2, 2);
>
> Uninitialized use of size2 right there.  And it's the same for the use of 
> .DEFERRED_INIT as the patch does:
>
> {
>   int size2;
>   size2 = .DEFERRED_INIT (size2, 2);
>   size2 = 4;
>   _1 = size2 > 5;
>   D.2240 = (int) _1;
> }
>
> Argument of the pseudo-function-call to .DEFERRED_INIT uninitialized -> 
> boom.
>
> You can't solve any of this by fiddling with how SSA rewriting behaves at 
> a large scale.  You need to change this and only this specific 
> interpretation of a use.  Or preferrably not generate it to start with.
>
>> IMO the foo_3 PHI makes no sense.  foo doesn't live beyond its block,
>> so there should be no loop-carried dependency here.
>> 
>> So yeah, if “it” meant that the fact that variables live too long,
>> then I agree that becomes a correctness issue with this feature,
>> rather than just an odd quirk.  Could we solve it by inserting
>> a clobber at the end of the variable's scope, or something like that?
>
> It would possibly make GCC not generate a PHI on this broken input, yes.  
> But why would that be an improvement?

Hopefully the above answers this.

>> > Agreed, I have made such change yesterday, and this work around the 
>> > current issue.
>> >
>> > temp = .DEFERRED_INIT (temp/WITH_SIZE_EXPR(temp), init_type)
>> >
>> > To:
>> >
>> > temp = .DEFERRED_INIT (SIZE_of_temp, init_type)
>> 
>> I think we're going round in circles here though.  The point of having
>> the undefined input was so that we would continue to warn about undefined
>> inputs.  The above issue doesn't seem like a good justification for
>> dropping that.
>
> If you want to rely on the undefined use for error reporting then you must 
> only generate an undefined use when there was one before, you can't just 
> insert new undefined uses.  I don't see how it could be otherwise, as soon 
> as you introduce new undefined uses you can and will run into GCC making 
> use of the undefinedness, not just this particular issue with lifetime and 
> PHI nodes which might be "solved" by clobbers.
>
> I think it's a chicken egg problem: you can't add undefined uses, for 
> which you need to know if there was one, but the facility is supposed to 
> help detecting if there is one to start with.

The idea is that .DEFERRED_INIT would naturally get optimised away by
DCE/DSE if the variable is provably initialised before it's used.

Thanks,
Richard

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

Hi, Michael,

> On Jul 1, 2021, at 9:40 AM, Michael Matz <matz@suse.de> wrote:
> 
> Hello,
> 
> I haven't followed this thread too closely, in particular I haven't seen 
> why the current form of the .DEFERRED_INIT call was chosen or suggested, 
> but it triggered my "well, that's obviously wrong" gut feeling; so sorry 
> for stating something which might be obvious to thread participants here.  
> Anyway:
> 
> On Thu, 1 Jul 2021, Richard Sandiford via Gcc-patches wrote:
> 
>>>> It's not a bug in SSA, it's at most a missed optimization there.
>>> 
>>> I still think that SSA cannot handle block-scoped variable correctly 
>>> in this case, it should not move the variable out side of the block 
>>> scope. -:)
>>> 
>>>> But with -ftrivial-auto-var-init it becomes a correctness issue.
>> 
>> I might have lost track of what “it” is here.  Do you mean the 
>> progation, or the fact that we have a PHI in the first place?
>> 
>> For:
>> 
>> unsigned int
>> f (int n)
>> {
>>  unsigned int res = 0;
>>  for (int i = 0; i < n; i += 1)
>>    {
>>      unsigned int foo;
>>      foo += 1;
> 
> Note that here foo is used uninitialized.  That is the thing from which 
> everything else follows.  Garbage in, garbage out.  It makes not too much 
> sense to argue that the generated PHI node on this loop (generated because 
> of the exposed-upward use of foo) is wrong, or right, or should better be 
> something else.  The input program was broken, so anything makes sense.

Yes, the PHI node was generated because of the exposed-upward use of “foo”.
this makes sense.

However, the PHI node should not be put out of the live scope of “foo”, this is wrong.

IMO, even though there is uninitialized variable, it’s not the excuse that compiler should
not maintain correct variable scopes.


> 
> That is the same with Qings shorter testcase:
> 
>  6   for (i = 0; i < a; i++) {
>  7     if (__extension__({int size2;
>  8         size2 = ART_INIT (size2, 2);
> 
> Uninitialized use of size2 right there.  And it's the same for the use of 
> .DEFERRED_INIT as the patch does:
> 
> {
>  int size2;
>  size2 = .DEFERRED_INIT (size2, 2);
>  size2 = 4;
>  _1 = size2 > 5;
>  D.2240 = (int) _1;
> }
> 
> Argument of the pseudo-function-call to .DEFERRED_INIT uninitialized -> 
> boom.
> 
> You can't solve any of this by fiddling with how SSA rewriting behaves at 
> a large scale.  You need to change this and only this specific 
> interpretation of a use.  Or preferrably not generate it to start with.
> 
>> IMO the foo_3 PHI makes no sense.  foo doesn't live beyond its block,
>> so there should be no loop-carried dependency here.
>> 
>> So yeah, if “it” meant that the fact that variables live too long,
>> then I agree that becomes a correctness issue with this feature,
>> rather than just an odd quirk.  Could we solve it by inserting
>> a clobber at the end of the variable's scope, or something like that?
> 
> It would possibly make GCC not generate a PHI on this broken input, yes.  
> But why would that be an improvement?

Being adding Clobbers at the end of the variable’s scope, the compiler will keep the correct
Variable scope during this stage, and prevent any more incorrect transformation from happening.
IMO  this should be a nice improvement.  -:)

Thanks.

Qing
> 
>>> Agreed, I have made such change yesterday, and this work around the 
>>> current issue.
>>> 
>>> temp = .DEFERRED_INIT (temp/WITH_SIZE_EXPR(temp), init_type)
>>> 
>>> To:
>>> 
>>> temp = .DEFERRED_INIT (SIZE_of_temp, init_type)
>> 
>> I think we're going round in circles here though.  The point of having
>> the undefined input was so that we would continue to warn about undefined
>> inputs.  The above issue doesn't seem like a good justification for
>> dropping that.
> 
> If you want to rely on the undefined use for error reporting then you must 
> only generate an undefined use when there was one before, you can't just 
> insert new undefined uses.  I don't see how it could be otherwise, as soon 
> as you introduce new undefined uses you can and will run into GCC making 
> use of the undefinedness, not just this particular issue with lifetime and 
> PHI nodes which might be "solved" by clobbers.
> 
> I think it's a chicken egg problem: you can't add undefined uses, for 
> which you need to know if there was one, but the facility is supposed to 
> help detecting if there is one to start with.
> 
> 
> Ciao,
> Michael.

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Michael Matz]

Hello,

On Thu, 1 Jul 2021, Richard Sandiford wrote:

> Well, it does feel like this is pressing the reset button on a thread
> whose message count is already in the high double figures.  There's a
> risk that restarting the discussion now is going to repeat a lot of the
> same ground, probably at similar length.  (But I realise that the sheer
> length of the thread is probably also the main reason for not having
> followed it closely. :-))

Yeah, I thought of not sending the mail because of that.  But it itched 
too itchy ;-)

> (2) continue to treat uses of uninitialised automatic variables as
>     (semantically) undefined behaviour
> 
> When this was discussed on the clang list, (2) was a key requirement
> and was instrumental in getting the feature accepted.
> ... 
> since the behaviour is at least deterministic.  But from a debugging QoI
> perspective, this should not happen.  We should generate the same code
> as for:
> 
>    int foo = <fill-pattern>;
>    if (flag)
>      foo = 1;
>    x = foo;
> 
> However, because of (2), we should still generate a 
> -Wmaybe-uninitialized warning for the “x = foo” assignment.
> 
> This is not a combination we support at the moment, so something needs 
> to give.  The question of course is what.

Nothing needs to give.  You add the initialization with <fill-pattern>, 
_without an uninitialized use_, i.e. just:

   foo = .deferred_init(pattern)

or the like.  Then you let the optimizers do their thing.  If 'foo' is 
provably initialized (or rather overwritten in this setting) on all paths 
from above init to each use, that init will be removed.  IOW: if the init 
remains then not all paths are provably initializing.  I.e. the warning 
code can be triggered based on the simple existence of the deferred_init 
initializer.

Basically the .deferred_init acts as a "here the scope starts" marker.  
That's indeed what you want, not the "here the scope ends" clobber.  If a 
use reaches the scope-start marker you have an (possibly) uninitialized 
use and warn.

(I'm obviously missing something because the above idea seems natural, so 
was probably already discussed and rejected, but why?)

> > Garbage in, garbage out.  It makes not too much sense to argue that
> > the generated PHI node on this loop (generated because of the
> > exposed-upward use of foo) is wrong, or right, or should better be
> > something else.  The input program was broken, so anything makes sense.
> 
> Yeah, without the new option it's GIGO.  But I think it's still possible
> to rank different forms of garbage output in terms of how self-consistent
> they are.
> 
> IMO it makes no sense to say that “foo” is upwards-exposed to somewhere
> where “foo” doesn't exist.  Using “foo_N(D)” doesn't have that problem,
> so I think it's possible to say that it's “better” garbage output. :-)
> 
> And the point is that with the new option, this is no longer garbage
> input as far as SSA is concerned: carrying the old value of “foo”
> across iterations would not be implementing the option correctly.
> How SSA handles this becomes a correctness issue.

I disagree.  The notion of "foo doesn't exist" simply doesn't exist (ugh!) 
in SSA; SSA is not the issue here.  The notion of ranges of where foo does 
or doesn't exist are scopes, that's orthogonal; so you want to encode that 
info somehow in a way compatible with SSA (!).  Doing that 
with explicit instructions is sensible (like marking scope endings with 
the clobber insn was sensible).  But those instructions simply need to not 
work against other invariants established and required by SSA.  Don't 
introduce new undefined uses and you're fine.

> > I think it's a chicken egg problem: you can't add undefined uses, for 
> > which you need to know if there was one, but the facility is supposed to 
> > help detecting if there is one to start with.
> 
> The idea is that .DEFERRED_INIT would naturally get optimised away by
> DCE/DSE if the variable is provably initialised before it's used.

Well, that's super.  So, why would you want or need the uninitialized use 
in the argument of .DEFERRED_INIT?


Ciao,
Michael.

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Richard Sandiford]

Michael Matz <matz@suse.de> writes:
> Hello,
>
> On Thu, 1 Jul 2021, Richard Sandiford wrote:
>
>> Well, it does feel like this is pressing the reset button on a thread
>> whose message count is already in the high double figures.  There's a
>> risk that restarting the discussion now is going to repeat a lot of the
>> same ground, probably at similar length.  (But I realise that the sheer
>> length of the thread is probably also the main reason for not having
>> followed it closely. :-))
>
> Yeah, I thought of not sending the mail because of that.  But it itched 
> too itchy ;-)
>
>> (2) continue to treat uses of uninitialised automatic variables as
>>     (semantically) undefined behaviour
>> 
>> When this was discussed on the clang list, (2) was a key requirement
>> and was instrumental in getting the feature accepted.
>> ... 
>> since the behaviour is at least deterministic.  But from a debugging QoI
>> perspective, this should not happen.  We should generate the same code
>> as for:
>> 
>>    int foo = <fill-pattern>;
>>    if (flag)
>>      foo = 1;
>>    x = foo;
>> 
>> However, because of (2), we should still generate a 
>> -Wmaybe-uninitialized warning for the “x = foo” assignment.
>> 
>> This is not a combination we support at the moment, so something needs 
>> to give.  The question of course is what.
>
> Nothing needs to give.  You add the initialization with <fill-pattern>, 
> _without an uninitialized use_, i.e. just:
>
>    foo = .deferred_init(pattern)
>
> or the like.  Then you let the optimizers do their thing.  If 'foo' is 
> provably initialized (or rather overwritten in this setting) on all paths 
> from above init to each use, that init will be removed.  IOW: if the init 
> remains then not all paths are provably initializing.  I.e. the warning 
> code can be triggered based on the simple existence of the deferred_init 
> initializer.
>
> Basically the .deferred_init acts as a "here the scope starts" marker.  
> That's indeed what you want, not the "here the scope ends" clobber.  If a 
> use reaches the scope-start marker you have an (possibly) uninitialized 
> use and warn.
>
> (I'm obviously missing something because the above idea seems natural, so 
> was probably already discussed and rejected, but why?)

The hope was that we wouldn't even need to treat DEFERRED_INIT specially
when analysing uninitialised uses.  The argument would be a standard
uninitialised use, just like any other.

But that was before this issue came up.  It also sounds like Qing is now
handling DEFERRED_INIT specially anyway.  (I've lost track of why,
but see above about reopening that discussion. :-))

So it sounds like that's indeed where we've ended up.  I don't think
the original principle was inherently bad though.

Thanks,
Richard

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Qing Zhao]

Hi, Richard,

> On Jul 1, 2021, at 11:23 AM, Richard Sandiford <richard.sandiford@arm.com> wrote:
> 
> Michael Matz <matz@suse.de> writes:
>> Hello,
>> 
>> On Thu, 1 Jul 2021, Richard Sandiford wrote:
>> 
>>> Well, it does feel like this is pressing the reset button on a thread
>>> whose message count is already in the high double figures.  There's a
>>> risk that restarting the discussion now is going to repeat a lot of the
>>> same ground, probably at similar length.  (But I realise that the sheer
>>> length of the thread is probably also the main reason for not having
>>> followed it closely. :-))
>> 
>> Yeah, I thought of not sending the mail because of that.  But it itched 
>> too itchy ;-)
>> 
>>> (2) continue to treat uses of uninitialised automatic variables as
>>>    (semantically) undefined behaviour
>>> 
>>> When this was discussed on the clang list, (2) was a key requirement
>>> and was instrumental in getting the feature accepted.
>>> ... 
>>> since the behaviour is at least deterministic.  But from a debugging QoI
>>> perspective, this should not happen.  We should generate the same code
>>> as for:
>>> 
>>>   int foo = <fill-pattern>;
>>>   if (flag)
>>>     foo = 1;
>>>   x = foo;
>>> 
>>> However, because of (2), we should still generate a 
>>> -Wmaybe-uninitialized warning for the “x = foo” assignment.
>>> 
>>> This is not a combination we support at the moment, so something needs 
>>> to give.  The question of course is what.
>> 
>> Nothing needs to give.  You add the initialization with <fill-pattern>, 
>> _without an uninitialized use_, i.e. just:
>> 
>>   foo = .deferred_init(pattern)
>> 
>> or the like.  Then you let the optimizers do their thing.  If 'foo' is 
>> provably initialized (or rather overwritten in this setting) on all paths 
>> from above init to each use, that init will be removed.  IOW: if the init 
>> remains then not all paths are provably initializing.  I.e. the warning 
>> code can be triggered based on the simple existence of the deferred_init 
>> initializer.
>> 
>> Basically the .deferred_init acts as a "here the scope starts" marker.  
>> That's indeed what you want, not the "here the scope ends" clobber.  If a 
>> use reaches the scope-start marker you have an (possibly) uninitialized 
>> use and warn.
>> 
>> (I'm obviously missing something because the above idea seems natural, so 
>> was probably already discussed and rejected, but why?)
> 
> The hope was that we wouldn't even need to treat DEFERRED_INIT specially
> when analysing uninitialised uses.  The argument would be a standard
> uninitialised use, just like any other.
> 
> But that was before this issue came up.  It also sounds like Qing is now
> handling DEFERRED_INIT specially anyway.  (I've lost track of why,
> but see above about reopening that discussion. :-))

The reason I handled DEFERRED_INIT specially in uninitialized warning analysis was because only relying
on the uninitialized argument in the .DEFERRED_INIT call was not enough for the analysis.

The special handlings was introduced in the 1st version of the patch. (I don’t remember the 
exact details on why it would not work without specially handling .DEFERRED_INIT)

Thanks

Qing
> 
> So it sounds like that's indeed where we've ended up.  I don't think
> the original principle was inherently bad though.
> 
> Thanks,
> Richard

Re: HELP!! How to inhibit optimizations applied to .DEFERRED_INIT argument?

[Richard Biener]

On July 1, 2021 4:40:11 PM GMT+02:00, Michael Matz <matz@suse.de> wrote:
>Hello,
>
>I haven't followed this thread too closely, in particular I haven't
>seen 
>why the current form of the .DEFERRED_INIT call was chosen or
>suggested, 
>but it triggered my "well, that's obviously wrong" gut feeling; so
>sorry 
>for stating something which might be obvious to thread participants
>here.  
>Anyway:
>
>On Thu, 1 Jul 2021, Richard Sandiford via Gcc-patches wrote:
>
>> >> It's not a bug in SSA, it's at most a missed optimization there.
>> >
>> > I still think that SSA cannot handle block-scoped variable
>correctly 
>> > in this case, it should not move the variable out side of the block
>
>> > scope. -:)
>> >
>> >>  But with -ftrivial-auto-var-init it becomes a correctness issue.
>> 
>> I might have lost track of what “it” is here.  Do you mean the 
>> progation, or the fact that we have a PHI in the first place?
>> 
>> For:
>> 
>> unsigned int
>> f (int n)
>> {
>>   unsigned int res = 0;
>>   for (int i = 0; i < n; i += 1)
>>     {
>>       unsigned int foo;
>>       foo += 1;
>
>Note that here foo is used uninitialized.  That is the thing from which
>
>everything else follows.  Garbage in, garbage out.  It makes not too
>much 
>sense to argue that the generated PHI node on this loop (generated
>because 
>of the exposed-upward use of foo) is wrong, or right, or should better
>be 
>something else.  The input program was broken, so anything makes sense.
>
>That is the same with Qings shorter testcase:
>
>  6   for (i = 0; i < a; i++) {
>  7     if (__extension__({int size2;
>  8         size2 = ART_INIT (size2, 2);
>
>Uninitialized use of size2 right there.  And it's the same for the use
>of 
>.DEFERRED_INIT as the patch does:
>
>{
>  int size2;
>  size2 = .DEFERRED_INIT (size2, 2);
>  size2 = 4;
>  _1 = size2 > 5;
>  D.2240 = (int) _1;
>}
>
>Argument of the pseudo-function-call to .DEFERRED_INIT uninitialized ->
>
>boom.
>
>You can't solve any of this by fiddling with how SSA rewriting behaves
>at 
>a large scale.  You need to change this and only this specific 
>interpretation of a use.  Or preferrably not generate it to start with.
>
>> IMO the foo_3 PHI makes no sense.  foo doesn't live beyond its block,
>> so there should be no loop-carried dependency here.
>> 
>> So yeah, if “it” meant that the fact that variables live too long,
>> then I agree that becomes a correctness issue with this feature,
>> rather than just an odd quirk.  Could we solve it by inserting
>> a clobber at the end of the variable's scope, or something like that?
>
>It would possibly make GCC not generate a PHI on this broken input,
>yes.  
>But why would that be an improvement?
>
>> > Agreed, I have made such change yesterday, and this work around the
>
>> > current issue.
>> >
>> > temp = .DEFERRED_INIT (temp/WITH_SIZE_EXPR(temp), init_type)
>> >
>> > To:
>> >
>> > temp = .DEFERRED_INIT (SIZE_of_temp, init_type)
>> 
>> I think we're going round in circles here though.  The point of
>having
>> the undefined input was so that we would continue to warn about
>undefined
>> inputs.  The above issue doesn't seem like a good justification for
>> dropping that.
>
>If you want to rely on the undefined use for error reporting then you
>must 
>only generate an undefined use when there was one before, you can't
>just 
>insert new undefined uses.  I don't see how it could be otherwise, as
>soon 
>as you introduce new undefined uses you can and will run into GCC
>making 
>use of the undefinedness, not just this particular issue with lifetime
>and 
>PHI nodes which might be "solved" by clobbers.
>
>I think it's a chicken egg problem: you can't add undefined uses, for 
>which you need to know if there was one, but the facility is supposed
>to 
>help detecting if there is one to start with.

But because of the place we insert DEFERED_INIT the use will always be uninitialized (or we generated wrong code), so the use is somewhat pointless and thus IMHO getting rid of it is the correct thing to do. 

Richard. 

>
>Ciao,
>Michael.