On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski <pins...@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 <rguent...@suse.de>
>wrote:
>> >
>> > On Wed, 30 Jun 2021, Qing Zhao wrote:
>> >
>> >>
>> >>
>> >> On Jun 30, 2021, at 2:46 AM, Richard Biener
><rguent...@suse.de<mailto:rguent...@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 <rguent...@suse.de<mailto:rguent...@suse.de>>
>> >> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
>Nuernberg,
>> >> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>> >>
>> >>
>> >
>> > --
>> > Richard Biener <rguent...@suse.de>
>> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
>Nuernberg,
>> > Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>
