FYI. Just committed this patch to gcc12 as:
https://gcc.gnu.org/pipermail/gcc-cvs/2022-January/359118.html Qing > On Jan 11, 2022, at 9:38 AM, Qing Zhao via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > >> On Jan 11, 2022, at 7:53 AM, Richard Biener <richard.guent...@gmail.com> >> wrote: >> >> On Tue, Jan 11, 2022 at 12:58 AM Qing Zhao <qing.z...@oracle.com> wrote: >>> >>> Hi, Richard, >>> >>> I splited the previous patch for “Enable -Wuninitialized + >>> -ftrivial-auto-var-init for address taken variables” into two separate >>> patches. >>> This is the first one >>> >>> This first patch is to fix (or work around ) PR103720, therefore it’s an >>> important change, and need to be go into GCC12. >>> At the same time, this patch is the preparation for the second patch that >>> will actually enable -Wuninitialized + -ftrivial-auto-var-init for address >>> taken variables. >>> >>> The reason I separate the previous patch into two is: most of the previous >>> concern was on the second part of the patch (the change in >>> tree-ssa-uninit.c), I don’t >>> want those concern prevent this first patch from being approved into GCC12. >>> >>> >>> In this part, I addressed your comments in gimplify.c : >>> >>> ===== >>> tree decl_name >>> + = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1, >>> + IDENTIFIER_POINTER (DECL_NAME (decl))); >>> >>> you need to deal with DECL_NAME being NULL. >>> ===== >>> >>> Please also see the detailed description below for the problem and solution >>> of this patch. >>> >>> This first patch has been bootstrapped and regressing tested on both X86 >>> and aarch64. >>> >>> Okay for GCC12? >> >> + >> + char *decl_name_anonymous = xasprintf ("D.%u", DECL_UID (decl)); >> + const char *decl_name_str = DECL_NAME (decl) >> + ? IDENTIFIER_POINTER (DECL_NAME (decl)) >> + : decl_name_anonymous; >> + tree decl_name >> + = build_string_literal (strlen (decl_name_str) + 1, >> + decl_name_str); >> >> please avoid the xasprintf in the case DECL_NAME is not NULL, I'd be happy >> with sth like >> >> tree decl_name; >> if (DECL_NAME (decl)) >> decl_name = build_string_literal (...); >> else >> { >> char *decl_name_anon = xasprintf (...); >> decl_name = build_string_literal (...); >> free (decl_name_anon); >> } >> >> otherwise the patch is OK to commit (just do the above change and >> re-test / push). > > Thanks for the comment, will fix this and commit the patch. > > Qing >> >> Thanks, >> Richard. >> >>> Thanks. >>> >>> Qing. >>> >>> >>> ================================= >>> >>> Change the 3rd parameter of function .DEFERRED_INIT from >>> IS_VLA to decl name. >>> >>> Currently, the 3rd parameter of function .DEFERRED_INIT is IS_VLA, which is >>> not needed at all; >>> >>> In this patch, we change the 3rd parameter from IS_VLA to the name of the >>> var >>> decl for the following purposes: >>> >>> 1. Fix (or work around) PR103720: >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720 >>> >>> As confirmed in PR103720, with the current definition of .DEFERRED_INIT, >>> >>> Dom transformed: >>> c$a$0_6 = .DEFERRED_INIT (8, 2, 0); >>> _1 = .DEFERRED_INIT (8, 2, 0); >>> >>> into: >>> c$a$0_6 = .DEFERRED_INIT (8, 2, 0); >>> _1 = c$a$0_6; >>> >>> which is incorrectly done due to Dom treating the two calls to const >>> function >>> .DEFERRED_INIT as the same call since all actual parameters are the same. >>> >>> The same issue has been exposed in PR102608 due to a different optimization >>> VN, >>> the fix for PR102608 is to specially handle call to .DEFERRED_INIT in VN to >>> exclude it from CSE. >>> >>> To fix PR103720, we could do the same as the fix to PR102608 to specially >>> handle call to .DEFERRED_INIT in Dom to exclude it from being optimized. >>> >>> However, in addition to Dom and VN, there should be other optimizations that >>> have the same issue as PR103720 or PR102608 (As I built Linux kernel with >>> -ftrivial-auto-var-init=zero -Werror, I noticed a bunch of bugos warnings). >>> >>> Other than identifying all the optimizations and specially handling call to >>> .DEFERRED_INIT in all these optimizations, changing the 3rd parameter of the >>> function .DEFERRED_INIT from IS_VLA to the name string of the var decl might >>> be a better workaround (or a fix). After this change, since the 3rd actual >>> parameter is the name string of the variable, different calls for different >>> variables will have different name strings as the 3rd actual, As a result, >>> the >>> optimization that previously treated the different calls to .DEFERRED_INIT >>> as >>> the same will be prevented. >>> >>> 2. Prepare for enabling -Wuninitialized + -ftrivail-auto-var-init for >>> address >>> taken variables. >>> >>> As discussion in the following thread: >>> >>> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html >>> >>> With the current implemenation of -ftrivial-auto-var-init and uninitialized >>> warning analysis, the uninitialized warning for an address taken auto >>> variable >>> might be missed since the variable is completely eliminated by optimization >>> and >>> replaced with a temporary variable in all the uses. >>> >>> In order to improve such situation, changing the 3rd parameter of the >>> function >>> .DEFERRED_INIT to the name string of the variable will provide necessary >>> information to uninitialized warning analysis to make the missing warning >>> possible. >>> >>> gcc/ChangeLog: >>> >>> 2022-01-10 qing zhao <qing.z...@oracle.com> >>> >>> * gimplify.c (gimple_add_init_for_auto_var): Delete the 3rd argument. >>> Change the 3rd argument of function .DEFERRED_INIT to the name of the >>> decl. >>> (gimplify_decl_expr): Delete the 3rd argument when call >>> gimple_add_init_for_auto_var. >>> * internal-fn.c (expand_DEFERRED_INIT): Update comments to reflect >>> the 3rd argument change of function .DEFERRED_INIT. >>> * tree-cfg.c (verify_gimple_call): Update comments and verification >>> to reflect the 3rd argument change of function .DEFERRED_INIT. >>> * tree-sra.c (generate_subtree_deferred_init): Delete the 3rd >>> argument. >>> (sra_modify_deferred_init): Change the 3rd argument of function >>> .DEFERRED_INIT to the name of the decl. >>> >>> gcc/testsuite/ChangeLog: >>> >>> 2022-01-10 qing zhao <qing.z...@oracle.com> >>> >>> * c-c++-common/auto-init-1.c: Adjust testcase to reflect the 3rd >>> argument change of function .DEFERRED_INIT. >>> * c-c++-common/auto-init-10.c: Likewise. >>> * c-c++-common/auto-init-11.c: Likewise. >>> * c-c++-common/auto-init-12.c: Likewise. >>> * c-c++-common/auto-init-13.c: Likewise. >>> * c-c++-common/auto-init-14.c: Likewise. >>> * c-c++-common/auto-init-15.c: Likewise. >>> * c-c++-common/auto-init-16.c: Likewise. >>> * c-c++-common/auto-init-2.c: Likewise. >>> * c-c++-common/auto-init-3.c: Likewise. >>> * c-c++-common/auto-init-4.c: Likewise. >>> * c-c++-common/auto-init-5.c: Likewise. >>> * c-c++-common/auto-init-6.c: Likewise. >>> * c-c++-common/auto-init-7.c: Likewise. >>> * c-c++-common/auto-init-8.c: Likewise. >>> * c-c++-common/auto-init-9.c: Likewise. >>> * c-c++-common/auto-init-esra.c: Likewise. >>> * c-c++-common/auto-init-padding-1.c: Likewise. >>> * gcc.target/aarch64/auto-init-2.c: Likewise. >>> >>> ******The complete patch is attached:
