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

[Richard Biener <rguenther@suse.de>]

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