Optionally use nameless IR types

[sepavloff]

Summary

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.

[rjmccall]

include/clang/Driver/Options.td

1735

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?

[sepavloff]

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.

[sepavloff]

include/clang/Driver/Options.td

1735

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.

[pcc]

Why is this a driver flag? This seems like it ought to be a cc1-only flag to me.

[rsmith]

include/clang/Driver/Options.td

644

Having "none" and "use" as values for the same option seems like a category error. always / never / auto would make some sense, though.

[sepavloff]

In 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.

[sepavloff]

include/clang/Driver/Options.td

644

Indeed. Will change it.

[pcc]

If the backend will be changed so that it will not depend on IR type names

Are you referring to 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.

[sepavloff]

In D43805#1029479, @pcc wrote:

If the backend will be changed so that it will not depend on IR type names

Are you referring to 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 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.

[sepavloff]

Use more consistent option names

[sepavloff]

Any feedback?

Thanks,
--Serge

include/clang/Driver/Options.td

 def fautolink : Flag <["-"], "fautolink">, Group<f_Group>;
 def fno_autolink : Flag <["-"], "fno-autolink">, Group<f_Group>,
   Flags<[DriverOption, CC1Option]>,
   HelpText<"Disable generation of linker directives for automatic library linking">;
 
 // C++ Coroutines TS
 def fcoroutines_ts : Flag <["-"], "fcoroutines-ts">, Group<f_Group>,
   Flags<[DriverOption, CC1Option]>,
   HelpText<"Enable support for the C++ Coroutines TS">;
 def fno_coroutines_ts : Flag <["-"], "fno-coroutines-ts">, Group<f_Group>,
   Flags<[DriverOption]>;
 
 def fembed_bitcode_EQ : Joined<["-"], "fembed-bitcode=">,
     Group<f_Group>, Flags<[DriverOption, CC1Option]>, MetaVarName<"<option>">,
     HelpText<"Embed LLVM bitcode (option: off, all, bitcode, marker)">;
 def fembed_bitcode : Flag<["-"], "fembed-bitcode">, Group<f_Group>,
   Alias<fembed_bitcode_EQ>, AliasArgs<["all"]>,
def iframeworkwithsysroot : JoinedOrSeparate<["-"], "iframeworkwithsysroot">,
   MetaVarName<"<directory>">, Flags<[CC1Option]>;
 def imacros : JoinedOrSeparate<["-", "--"], "imacros">, Group<clang_i_Group>, Flags<[CC1Option]>,
   HelpText<"Include macros from file before parsing">, MetaVarName<"<file>">;
 def image__base : Separate<["-"], "image_base">;
 def include_ : JoinedOrSeparate<["-", "--"], "include">, Group<clang_i_Group>, EnumName<"include">,
     MetaVarName<"<file>">, HelpText<"Include file before parsing">, Flags<[CC1Option]>;
 def include_pch : Separate<["-"], "include-pch">, Group<clang_i_Group>, Flags<[CC1Option]>,
   HelpText<"Include precompiled header file">, MetaVarName<"<file>">;
+def ir_type_names_EQ : Joined<["--"], "ir-type-names=">,
+  Flags<[CC1Option, HelpHidden]>,
+  HelpText<"Whether to use IR type names (option: none, use)">,
+  Values<"none,use">;
 def relocatable_pch : Flag<["-", "--"], "relocatable-pch">, Flags<[CC1Option]>,
   HelpText<"Whether to build a relocatable precompiled header">;
 def verify_pch : Flag<["-"], "verify-pch">, Group<Action_Group>, Flags<[CC1Option]>,
   HelpText<"Load and verify that a pre-compiled header file is not stale">;
 def init : Separate<["-"], "init">;
 def install__name : Separate<["-"], "install_name">;
 def iprefix : JoinedOrSeparate<["-"], "iprefix">, Group<clang_i_Group>, Flags<[CC1Option]>,
   HelpText<"Set the -iwithprefix/-iwithprefixbefore prefix">, MetaVarName<"<dir>">;

include/clang/Frontend/CodeGenOptions.h

public:
 
   enum EmbedBitcodeKind {
     Embed_Off,      // No embedded bitcode.
     Embed_All,      // Embed both bitcode and commandline in the output.
     Embed_Bitcode,  // Embed just the bitcode in the output.
     Embed_Marker    // Embed a marker as a placeholder for bitcode.
   };
 
+  /// Should LLVM types be given human-readable names?
+  ///
+  enum class IRNameKind {
+    Unspecified,    ///< No special option was passed.
+    None,           ///< No type names.
+    Use             ///< Types must be named.
+  };
+
   /// The code model to use (-mcmodel).
   std::string CodeModel;
 
   /// The filename with path we use for coverage data files. The runtime
   /// allows further manipulation with the GCOV_PREFIX and GCOV_PREFIX_STRIP
   /// environment variables.
   std::string CoverageDataFile;
 

include/clang/Frontend/CodeGenOptions.def

CODEGENOPT(EmitDeclMetadata  , 1, 0) ///< Emit special metadata indicating what
                                      ///< Only useful when running CodeGen as a
                                      ///< subroutine.
 CODEGENOPT(EmitGcovArcs      , 1, 0) ///< Emit coverage data files, aka. GCDA.
 CODEGENOPT(EmitGcovNotes     , 1, 0) ///< Emit coverage "notes" files, aka GCNO.
 CODEGENOPT(EmitOpenCLArgMetadata , 1, 0) ///< Emit OpenCL kernel arg metadata.
 CODEGENOPT(EmulatedTLS       , 1, 0) ///< Set when -femulated-tls is enabled.
 /// \brief Embed Bitcode mode (off/all/bitcode/marker).
 ENUM_CODEGENOPT(EmbedBitcode, EmbedBitcodeKind, 2, Embed_Off)
+
+/// Should LLVM IR types be given human-readable names?
+/// \see CodeGenOptions::IRNameKind.
+ENUM_CODEGENOPT(IRTypeNames, IRNameKind, 4, IRNameKind::Unspecified)
+
 CODEGENOPT(ForbidGuardVariables , 1, 0) ///< Issue errors if C++ guard variables
                                         ///< are required.
 CODEGENOPT(FunctionSections  , 1, 0) ///< Set when -ffunction-sections is enabled.
 CODEGENOPT(InstrumentFunctions , 1, 0) ///< Set when -finstrument-functions is
                                        ///< enabled.
 CODEGENOPT(InstrumentFunctionsAfterInlining , 1, 0) ///< Set when
                           ///< -finstrument-functions-after-inlining is enabled.
 CODEGENOPT(InstrumentFunctionEntryBare , 1, 0) ///< Set when

lib/CodeGen/CodeGenAction.cpp

CodeGenAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
   CoverageSourceInfo *CoverageInfo = nullptr;
   // Add the preprocessor callback only when the coverage mapping is generated.
   if (CI.getCodeGenOpts().CoverageMapping) {
     CoverageInfo = new CoverageSourceInfo;
     CI.getPreprocessor().addPPCallbacks(
                                     std::unique_ptr<PPCallbacks>(CoverageInfo));
   }
 
+  // Specify names for IR types in the cases:
+  // - if it requested explicitly, by option '--ir-type-names=use',
+  // - if compilation produces IR representation, or
+  // - in Debug build.
+  if (CI.getCodeGenOpts().getIRTypeNames()
+                                   == CodeGenOptions::IRNameKind::Unspecified) {
+#ifdef NDEBUG
+    if ((BA == Backend_EmitBC ||
+         BA == Backend_EmitLL ||
+         (BA == Backend_EmitObj &&
+          CI.getCodeGenOpts().getEmbedBitcode() != CodeGenOptions::Embed_Off) ||
+         !LinkModules.empty()))
+#endif
+      CI.getCodeGenOpts().setIRTypeNames(CodeGenOptions::IRNameKind::Use);
+  }
+
   std::unique_ptr<BackendConsumer> Result(new BackendConsumer(
       BA, CI.getDiagnostics(), CI.getHeaderSearchOpts(),
       CI.getPreprocessorOpts(), CI.getCodeGenOpts(), CI.getTargetOpts(),
       CI.getLangOpts(), CI.getFrontendOpts().ShowTimers, InFile,
       std::move(LinkModules), std::move(OS), *VMContext, CoverageInfo));
   BEConsumer = Result.get();
 
   // Enable generating macro debug info only when debug info is not disabled and

lib/CodeGen/CodeGenTypes.cpp

 
 const CodeGenOptions &CodeGenTypes::getCodeGenOpts() const {
   return CGM.getCodeGenOpts();
 }
 
 void CodeGenTypes::addRecordTypeName(const RecordDecl *RD,
                                      llvm::StructType *Ty,
                                      StringRef suffix) {
+  if (getCodeGenOpts().getIRTypeNames() == CodeGenOptions::IRNameKind::None)
+    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 (RD->getIdentifier()) {
     // FIXME: We should not have to check for a null decl context here.
     // Right now we do it because the implicit Obj-C decls don't have one.
     if (RD->getDeclContext())
       RD->printQualifiedName(OS);
     else

lib/Driver/ToolChains/Clang.cpp

if (C.getDriver().embedBitcodeInObject() && !C.getDriver().isUsingLTO() &&
     // Add flags implied by -fembed-bitcode.
     Args.AddLastArg(CmdArgs, options::OPT_fembed_bitcode_EQ);
     // Disable all llvm IR level optimizations.
     CmdArgs.push_back("-disable-llvm-passes");
   }
   if (C.getDriver().embedBitcodeMarkerOnly() && !C.getDriver().isUsingLTO())
     CmdArgs.push_back("-fembed-bitcode=marker");
 
+  Args.AddLastArg(CmdArgs, options::OPT_ir_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.
   if (!C.isForDiagnostics())
     CmdArgs.push_back("-disable-free");
 
 #ifdef NDEBUG
   const bool IsAssertBuild = false;

lib/Frontend/CompilerInvocation.cpp

for (const auto &A : Args) {
         StringRef ArgStr(arg);
         Opts.CmdArgs.insert(Opts.CmdArgs.end(), ArgStr.begin(), ArgStr.end());
         // using \00 to seperate each commandline options.
         Opts.CmdArgs.push_back('\0');
       }
     }
   }
 
+  if (Arg *A = Args.getLastArg(OPT_ir_type_names_EQ)) {
+    StringRef Name = A->getValue();
+    auto Info = llvm::StringSwitch<CodeGenOptions::IRNameKind>(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 =
       Args.hasArg(OPT_finstrument_functions_after_inlining);
   Opts.InstrumentFunctionEntryBare =
       Args.hasArg(OPT_finstrument_function_entry_bare);
   Opts.XRayInstrumentFunctions = Args.hasArg(OPT_fxray_instrument);
   Opts.XRayAlwaysEmitCustomEvents =

test/CodeGenCXX/pr29160.cpp

 // 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)
 
 template <typename... Ts>
 struct Foo {
   template <typename... T>
   static void ignore() {}
   Foo() { ignore<Ts...>(); }
+  struct ABC {};
 };
 
 struct Base {
   Base();
   ~Base();
 };
 
 #define STAMP(thiz, prev) using thiz = Foo< \

 STAMP(L, K);
 STAMP(M, L);
 STAMP(N, M);
 STAMP(O, N);
 STAMP(P, O);
 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

test/CodeGenCXX/type-names.cpp

+// 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<typename T> struct C2 {
+  C1 a;
+  T b;
+  struct Inner {
+  };
+};
+
+C2<short> var_C2_a;
+C2<long long> var_C2_b;
+C2<int>::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 = &var_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<int>::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* }