[PATCH] D43805: Optionally use nameless IR types
sepavloff updated this revision to Diff 137336. sepavloff added a comment. Use more consistent option names Repository: rC Clang https://reviews.llvm.org/D43805 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def include/clang/Frontend/CodeGenOptions.h lib/CodeGen/CodeGenAction.cpp lib/CodeGen/CodeGenTypes.cpp lib/Driver/ToolChains/Clang.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGenCXX/pr29160.cpp test/CodeGenCXX/type-names.cpp Index: test/CodeGenCXX/type-names.cpp === --- /dev/null +++ test/CodeGenCXX/type-names.cpp @@ -0,0 +1,65 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -fir-type-names=always -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -fir-type-names=never -o - %s | FileCheck %s --check-prefix=UNNAMED + +struct C1 { + int a; + int *b; +}; + +C1 var_C1_a; +C1 var_C1_b[10]; +C1 *var_C1_c; +int (*var_C1_d)(const C1 &); + +struct C1A { + int a; + int *b; +}; + +C1A var_C1A_a; + +template struct C2 { + C1 a; + T b; + struct Inner { + }; +}; + +C2 var_C2_a; +C2 var_C2_b; +C2::Inner var_C2_c; + +struct C3 { + double a; + struct C4 { +int a; +float b; + }; +}; + +C3::C4 var_c4; + +namespace { +struct C5 { + int *a; +}; +} + +C5 var_C5_a; +void *var_C5_b = _C5_a; + +// CHECK: %struct.C1 = type { i32, i32* } +// CHECK: %struct.C1A = type { i32, i32* } +// CHECK: %struct.C2 = type { %struct.C1, i16 } +// CHECK: %struct.C2.0 = type { %struct.C1, i64 } +// CHECK: %"struct.C2::Inner" = type { i8 } +// CHECK: %"struct.C3::C4" = type { i32, float } +// CHECK: %"struct.(anonymous namespace)::C5" = type { i32* } + +// UNNAMED: %0 = type { i32, i32* } +// UNNAMED: %1 = type { i32, i32* } +// UNNAMED: %2 = type { %0, i16 } +// UNNAMED: %3 = type { %0, i64 } +// UNNAMED: %4 = type { i8 } +// UNNAMED: %5 = type { i32, float } +// UNNAMED: %6 = type { i32* } Index: test/CodeGenCXX/pr29160.cpp === --- test/CodeGenCXX/pr29160.cpp +++ test/CodeGenCXX/pr29160.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -std=c++11 -triple i686-linux-gnu %s -o /dev/null -S -emit-llvm +// RUN: %clang_cc1 -std=c++11 -triple i686-linux-gnu -DNAMELESS -fir-type-names=never %s -o /dev/null -S -emit-llvm // // This test's failure mode is running ~forever. (For some value of "forever" // that's greater than 25 minutes on my machine) @@ -8,6 +9,7 @@ template static void ignore() {} Foo() { ignore(); } + struct ABC {}; }; struct Base { @@ -39,3 +41,9 @@ STAMP(Q, P); int main() { Q q; } + +#ifdef NAMELESS +// Without '-fir-type-names=none' compiler tries to create name for Q::ABC, +// which is really huge, so compilation never ends. +Q::ABC var; +#endif Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -804,6 +804,20 @@ } } + if (Arg *A = Args.getLastArg(OPT_fir_type_names_EQ)) { +StringRef Name = A->getValue(); +auto Info = llvm::StringSwitch(Name) +.Case("auto", CodeGenOptions::IRNames_Auto) +.Case("never", CodeGenOptions::IRNames_Never) +.Case("always", CodeGenOptions::IRNames_Always) +.Default(-1); +if (Info == -1) { + Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name; + Success = false; +} else + Opts.setIRTypeNames(static_cast(Info)); + } + Opts.PreserveVec3Type = Args.hasArg(OPT_fpreserve_vec3_type); Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions); Opts.InstrumentFunctionsAfterInlining = Index: lib/Driver/ToolChains/Clang.cpp === --- lib/Driver/ToolChains/Clang.cpp +++ lib/Driver/ToolChains/Clang.cpp @@ -3279,6 +3279,8 @@ if (C.getDriver().embedBitcodeMarkerOnly() && !C.getDriver().isUsingLTO()) CmdArgs.push_back("-fembed-bitcode=marker"); + Args.AddLastArg(CmdArgs, options::OPT_fir_type_names_EQ); + // We normally speed up the clang process a bit by skipping destructors at // exit, but when we're generating diagnostics we can rely on some of the // cleanup. Index: lib/CodeGen/CodeGenTypes.cpp === --- lib/CodeGen/CodeGenTypes.cpp +++ lib/CodeGen/CodeGenTypes.cpp @@ -51,10 +51,13 @@ void CodeGenTypes::addRecordTypeName(const RecordDecl *RD, llvm::StructType *Ty, StringRef suffix) { + if (getCodeGenOpts().getIRTypeNames() == CodeGenOptions::IRNames_Never) +return; + SmallString<256> TypeName; llvm::raw_svector_ostream OS(TypeName); OS << RD->getKindName() << '.'; - + // Name the codegen type after the typedef name // if there is no tag type name available if
[PATCH] D43805: Optionally use nameless IR types
sepavloff added a comment. In https://reviews.llvm.org/D43805#1029479, @pcc wrote: > > If the backend will be changed so that it will not depend on IR type names > > Are you referring to https://reviews.llvm.org/D43199? If so it seems to me > that this should be a cc1 flag that defaults to whether `-flto=thin` is > passed. In any case it seems like a bad idea to deliberately generate > different code depending on whether we were compiled with NDEBUG. No, I try to implement alternative approach, to solve the problem targeted in https://reviews.llvm.org/D40508. If IR type names are only for human readability, than using them in opaque type resolution does not look reasonable. Probably, more correct way is to make type merge only as a side effect of merge of other entities, which may have "real" names, that is functions and variables. Repository: rC Clang https://reviews.llvm.org/D43805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43805: Optionally use nameless IR types
pcc added a comment. > If the backend will be changed so that it will not depend on IR type names Are you referring to https://reviews.llvm.org/D43199? If so it seems to me that this should be a cc1 flag that defaults to whether `-flto=thin` is passed. In any case it seems like a bad idea to deliberately generate different code depending on whether we were compiled with NDEBUG. Repository: rC Clang https://reviews.llvm.org/D43805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43805: Optionally use nameless IR types
sepavloff added inline comments. Comment at: include/clang/Driver/Options.td:644 + HelpText<"Whether to use IR type names (option: auto, none, use)">, + Values<"auto,none,use">; + rsmith wrote: > Having "none" and "use" as values for the same option seems like a category > error. always / never / auto would make some sense, though. Indeed. Will change it. Repository: rC Clang https://reviews.llvm.org/D43805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43805: Optionally use nameless IR types
sepavloff marked an inline comment as done. sepavloff added a comment. In https://reviews.llvm.org/D43805#1029455, @pcc wrote: > Why is this a driver flag? This seems like it ought to be a cc1-only flag to > me. Yous are right, this is more like development option. But having the driver flag allows using compiler in builds with either option. The option was hidden, probably it should be make hidden again. If the backend will be changed so that it will not depend on IR type names, the default mode can be set to nameless types and the option can become -cc1 only. Repository: rC Clang https://reviews.llvm.org/D43805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43805: Optionally use nameless IR types
rsmith added inline comments. Comment at: include/clang/Driver/Options.td:644 + HelpText<"Whether to use IR type names (option: auto, none, use)">, + Values<"auto,none,use">; + Having "none" and "use" as values for the same option seems like a category error. always / never / auto would make some sense, though. Repository: rC Clang https://reviews.llvm.org/D43805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43805: Optionally use nameless IR types
pcc added a comment. Why is this a driver flag? This seems like it ought to be a cc1-only flag to me. Repository: rC Clang https://reviews.llvm.org/D43805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D43805: Optionally use nameless IR types
Any feedback? Thanks, --Serge 2018-02-28 0:02 GMT+07:00 Serge Pavlov via Phabricator < revi...@reviews.llvm.org>: > sepavloff marked an inline comment as done. > sepavloff added inline comments. > > > > Comment at: include/clang/Driver/Options.td:1735 > + HelpText<"Whether to use IR type names (option: none, use)">, > + Values<"none,use">; > def relocatable_pch : Flag<["-", "--"], "relocatable-pch">, > Flags<[CC1Option]>, > > rjmccall wrote: > > This is an unusual spelling for the option in a number of ways: > > - We generally don't use `--` options; I think all the ones we have > are strictly for legacy support. > > - A lot of similar options are in the `-f` or `-m` namespaces, > although that's not as consistent and we could reasonably make this an > exception. > > - `-foo=bar` options are generally used for options that are expected > to take a variety of different values; this seems basically boolean. Are > you expecting future growth here? > The option is in fact a three-state one, the third 'value' is absence of > the option. In this case the option value is calculated from the type of > action (produce ll file or not) and from the type of build (in debug builds > types are named by default). To avoid misunderstanding I added new value, > 'auto' for this purpose. > > The option was renamed to `-fir-type-names=` and it is not hidden anymore. > > > Repository: > rC Clang > > https://reviews.llvm.org/D43805 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43805: Optionally use nameless IR types
sepavloff marked an inline comment as done. sepavloff added inline comments. Comment at: include/clang/Driver/Options.td:1735 + HelpText<"Whether to use IR type names (option: none, use)">, + Values<"none,use">; def relocatable_pch : Flag<["-", "--"], "relocatable-pch">, Flags<[CC1Option]>, rjmccall wrote: > This is an unusual spelling for the option in a number of ways: > - We generally don't use `--` options; I think all the ones we have are > strictly for legacy support. > - A lot of similar options are in the `-f` or `-m` namespaces, although > that's not as consistent and we could reasonably make this an exception. > - `-foo=bar` options are generally used for options that are expected to > take a variety of different values; this seems basically boolean. Are you > expecting future growth here? The option is in fact a three-state one, the third 'value' is absence of the option. In this case the option value is calculated from the type of action (produce ll file or not) and from the type of build (in debug builds types are named by default). To avoid misunderstanding I added new value, 'auto' for this purpose. The option was renamed to `-fir-type-names=` and it is not hidden anymore. Repository: rC Clang https://reviews.llvm.org/D43805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43805: Optionally use nameless IR types
sepavloff updated this revision to Diff 136087. sepavloff added a comment. Updated patch - command line option was renamed to `-fir-type-names=`, - this option is not hidden, as most options in `-f` namespace, - new value, `auto` was added to possible values of this option. Repository: rC Clang https://reviews.llvm.org/D43805 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def include/clang/Frontend/CodeGenOptions.h lib/CodeGen/CodeGenAction.cpp lib/CodeGen/CodeGenTypes.cpp lib/Driver/ToolChains/Clang.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGenCXX/pr29160.cpp test/CodeGenCXX/type-names.cpp Index: test/CodeGenCXX/type-names.cpp === --- /dev/null +++ test/CodeGenCXX/type-names.cpp @@ -0,0 +1,65 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -fir-type-names=use -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -fir-type-names=none -o - %s | FileCheck %s --check-prefix=UNNAMED + +struct C1 { + int a; + int *b; +}; + +C1 var_C1_a; +C1 var_C1_b[10]; +C1 *var_C1_c; +int (*var_C1_d)(const C1 &); + +struct C1A { + int a; + int *b; +}; + +C1A var_C1A_a; + +template struct C2 { + C1 a; + T b; + struct Inner { + }; +}; + +C2 var_C2_a; +C2 var_C2_b; +C2::Inner var_C2_c; + +struct C3 { + double a; + struct C4 { +int a; +float b; + }; +}; + +C3::C4 var_c4; + +namespace { +struct C5 { + int *a; +}; +} + +C5 var_C5_a; +void *var_C5_b = _C5_a; + +// CHECK: %struct.C1 = type { i32, i32* } +// CHECK: %struct.C1A = type { i32, i32* } +// CHECK: %struct.C2 = type { %struct.C1, i16 } +// CHECK: %struct.C2.0 = type { %struct.C1, i64 } +// CHECK: %"struct.C2::Inner" = type { i8 } +// CHECK: %"struct.C3::C4" = type { i32, float } +// CHECK: %"struct.(anonymous namespace)::C5" = type { i32* } + +// UNNAMED: %0 = type { i32, i32* } +// UNNAMED: %1 = type { i32, i32* } +// UNNAMED: %2 = type { %0, i16 } +// UNNAMED: %3 = type { %0, i64 } +// UNNAMED: %4 = type { i8 } +// UNNAMED: %5 = type { i32, float } +// UNNAMED: %6 = type { i32* } Index: test/CodeGenCXX/pr29160.cpp === --- test/CodeGenCXX/pr29160.cpp +++ test/CodeGenCXX/pr29160.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -std=c++11 -triple i686-linux-gnu %s -o /dev/null -S -emit-llvm +// RUN: %clang_cc1 -std=c++11 -triple i686-linux-gnu -DNAMELESS -fir-type-names=none %s -o /dev/null -S -emit-llvm // // This test's failure mode is running ~forever. (For some value of "forever" // that's greater than 25 minutes on my machine) @@ -8,6 +9,7 @@ template static void ignore() {} Foo() { ignore(); } + struct ABC {}; }; struct Base { @@ -39,3 +41,9 @@ STAMP(Q, P); int main() { Q q; } + +#ifdef NAMELESS +// Without '-fir-type-names=none' compiler tries to create name for Q::ABC, +// which is really huge, so compilation never ends. +Q::ABC var; +#endif Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -804,6 +804,20 @@ } } + if (Arg *A = Args.getLastArg(OPT_fir_type_names_EQ)) { +StringRef Name = A->getValue(); +auto Info = llvm::StringSwitch(Name) +.Case("auto", CodeGenOptions::IRNames_Auto) +.Case("none", CodeGenOptions::IRNames_None) +.Case("use", CodeGenOptions::IRNames_Use) +.Default(-1); +if (Info == -1) { + Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name; + Success = false; +} else + Opts.setIRTypeNames(static_cast(Info)); + } + Opts.PreserveVec3Type = Args.hasArg(OPT_fpreserve_vec3_type); Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions); Opts.InstrumentFunctionsAfterInlining = Index: lib/Driver/ToolChains/Clang.cpp === --- lib/Driver/ToolChains/Clang.cpp +++ lib/Driver/ToolChains/Clang.cpp @@ -3279,6 +3279,8 @@ if (C.getDriver().embedBitcodeMarkerOnly() && !C.getDriver().isUsingLTO()) CmdArgs.push_back("-fembed-bitcode=marker"); + Args.AddLastArg(CmdArgs, options::OPT_fir_type_names_EQ); + // We normally speed up the clang process a bit by skipping destructors at // exit, but when we're generating diagnostics we can rely on some of the // cleanup. Index: lib/CodeGen/CodeGenTypes.cpp === --- lib/CodeGen/CodeGenTypes.cpp +++ lib/CodeGen/CodeGenTypes.cpp @@ -51,10 +51,13 @@ void CodeGenTypes::addRecordTypeName(const RecordDecl *RD, llvm::StructType *Ty, StringRef suffix) { + if (getCodeGenOpts().getIRTypeNames() == CodeGenOptions::IRNames_None) +return; + SmallString<256> TypeName; llvm::raw_svector_ostream
[PATCH] D43805: Optionally use nameless IR types
rjmccall added inline comments. Comment at: include/clang/Driver/Options.td:1735 + HelpText<"Whether to use IR type names (option: none, use)">, + Values<"none,use">; def relocatable_pch : Flag<["-", "--"], "relocatable-pch">, Flags<[CC1Option]>, This is an unusual spelling for the option in a number of ways: - We generally don't use `--` options; I think all the ones we have are strictly for legacy support. - A lot of similar options are in the `-f` or `-m` namespaces, although that's not as consistent and we could reasonably make this an exception. - `-foo=bar` options are generally used for options that are expected to take a variety of different values; this seems basically boolean. Are you expecting future growth here? Repository: rC Clang https://reviews.llvm.org/D43805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43805: Optionally use nameless IR types
sepavloff created this revision. sepavloff added reviewers: rsmith, rjmccall. Type in the LLVM IR may have names but only for the purpose of human readability (see discussions in https://reviews.llvm.org/D40567, http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20171127/210816.html and http://lists.llvm.org/pipermail/llvm-dev/2017-December/119585.html). In the case when the resulting IR is not proposed for reading, for instance, when compilation produces machine code, the type names are waste of memory. In some cases, when types are nested in other types, the memory expenses may be really large. This change implements new clang option, '--ir-type-names=', which controls if IR types should be given human readable names. The option may have values 'use' or 'none', which turn names on or off correspondently. If no such option was specified, compiler assign names when output may be read by a human, namely when IR is saved beyond compilation or in debug builds. Repository: rC Clang https://reviews.llvm.org/D43805 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def include/clang/Frontend/CodeGenOptions.h lib/CodeGen/CodeGenAction.cpp lib/CodeGen/CodeGenTypes.cpp lib/Driver/ToolChains/Clang.cpp lib/Frontend/CompilerInvocation.cpp test/CodeGenCXX/pr29160.cpp test/CodeGenCXX/type-names.cpp Index: test/CodeGenCXX/type-names.cpp === --- /dev/null +++ test/CodeGenCXX/type-names.cpp @@ -0,0 +1,65 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm --ir-type-names=use -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm --ir-type-names=none -o - %s | FileCheck %s --check-prefix=UNNAMED + +struct C1 { + int a; + int *b; +}; + +C1 var_C1_a; +C1 var_C1_b[10]; +C1 *var_C1_c; +int (*var_C1_d)(const C1 &); + +struct C1A { + int a; + int *b; +}; + +C1A var_C1A_a; + +template struct C2 { + C1 a; + T b; + struct Inner { + }; +}; + +C2 var_C2_a; +C2 var_C2_b; +C2::Inner var_C2_c; + +struct C3 { + double a; + struct C4 { +int a; +float b; + }; +}; + +C3::C4 var_c4; + +namespace { +struct C5 { + int *a; +}; +} + +C5 var_C5_a; +void *var_C5_b = _C5_a; + +// CHECK: %struct.C1 = type { i32, i32* } +// CHECK: %struct.C1A = type { i32, i32* } +// CHECK: %struct.C2 = type { %struct.C1, i16 } +// CHECK: %struct.C2.0 = type { %struct.C1, i64 } +// CHECK: %"struct.C2::Inner" = type { i8 } +// CHECK: %"struct.C3::C4" = type { i32, float } +// CHECK: %"struct.(anonymous namespace)::C5" = type { i32* } + +// UNNAMED: %0 = type { i32, i32* } +// UNNAMED: %1 = type { i32, i32* } +// UNNAMED: %2 = type { %0, i16 } +// UNNAMED: %3 = type { %0, i64 } +// UNNAMED: %4 = type { i8 } +// UNNAMED: %5 = type { i32, float } +// UNNAMED: %6 = type { i32* } Index: test/CodeGenCXX/pr29160.cpp === --- test/CodeGenCXX/pr29160.cpp +++ test/CodeGenCXX/pr29160.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -std=c++11 -triple i686-linux-gnu %s -o /dev/null -S -emit-llvm +// RUN: %clang_cc1 -std=c++11 -triple i686-linux-gnu -DNAMELESS --ir-type-names=none %s -o /dev/null -S -emit-llvm // // This test's failure mode is running ~forever. (For some value of "forever" // that's greater than 25 minutes on my machine) @@ -8,6 +9,7 @@ template static void ignore() {} Foo() { ignore(); } + struct ABC {}; }; struct Base { @@ -39,3 +41,9 @@ STAMP(Q, P); int main() { Q q; } + +#ifdef NAMELESS +// Without '--ir-type-names=none' compiler tries to create name for Q::ABC, +// which is really huge, so compilation never ends. +Q::ABC var; +#endif Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -804,6 +804,19 @@ } } + if (Arg *A = Args.getLastArg(OPT_ir_type_names_EQ)) { +StringRef Name = A->getValue(); +auto Info = llvm::StringSwitch(Name) +.Case("none", CodeGenOptions::IRNameKind::None) +.Case("use", CodeGenOptions::IRNameKind::Use) +.Default(CodeGenOptions::IRNameKind::Unspecified); +if (Info == CodeGenOptions::IRNameKind::Unspecified) { + Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name; + Success = false; +} else + Opts.setIRTypeNames(Info); + } + Opts.PreserveVec3Type = Args.hasArg(OPT_fpreserve_vec3_type); Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions); Opts.InstrumentFunctionsAfterInlining = Index: lib/Driver/ToolChains/Clang.cpp === --- lib/Driver/ToolChains/Clang.cpp +++ lib/Driver/ToolChains/Clang.cpp @@ -3279,6 +3279,8 @@ if (C.getDriver().embedBitcodeMarkerOnly() && !C.getDriver().isUsingLTO()) CmdArgs.push_back("-fembed-bitcode=marker"); + Args.AddLastArg(CmdArgs,