Hi, Andrew, Thanks a lot for your explanation.
On Jun 30, 2021, at 12:20 PM, 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, 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 <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)
