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