[DebugInfo] Add -fno-ctor-homing for as counterpart to -fuse-ctor-homing

[akhuang]

Summary

Add an opt out flag for constructor homing.

[akhuang]

add actual flag

[dblaikie]

Looks alright to me. (bit awkward having a documented cc1 option - since the cc1 options aren't meant to be user facing - hadn't really thought about/noticed that when reviewing the documentation originally in D95911)

[akhuang]

In D106582#2897599, @dblaikie wrote:

Looks alright to me. (bit awkward having a documented cc1 option - since the cc1 options aren't meant to be user facing - hadn't really thought about/noticed that when reviewing the documentation originally in D95911)

Oh, true. Since we're making ctor homing default maybe I should just make them user facing flags.

[dblaikie]

In D106582#2897678, @akhuang wrote:

In D106582#2897599, @dblaikie wrote:

Looks alright to me. (bit awkward having a documented cc1 option - since the cc1 options aren't meant to be user facing - hadn't really thought about/noticed that when reviewing the documentation originally in D95911)

Oh, true. Since we're making ctor homing default maybe I should just make them user facing flags.

I have mixed feelings - on the one hand, yeah, we're changing the behavior of -fno-standalone-debug and giving people a tool to undo that change if it's problematic is good (less chance we have to churn through reverting the change, fixing, reapplying, etc, if there's a way to opt-out, at least in the short term). But equally I don't want to proliferate various rather arbitrary buckets of debug info type homing - I really want to encourage people to think of it as a fairly all-or-nothing situation, because I don't think there's a good way to explain/improve/modify/use the feature if it's "whatever is good enough for my use case". Especially now that there's the standalone_debug attribute that can be used to fix particular use cases where a type is needed across a -g0 boundary.

So... eh. Could go either way - your call, really. Though if we make it a full driver option, maybe a caveat that we may remove the functionality in the future (but that'll mean probably having to leave the flag in as a no-op/with some deprecation warning for some time after removing the functionality).

[akhuang]

In D106582#2897815, @dblaikie wrote:

In D106582#2897678, @akhuang wrote:

In D106582#2897599, @dblaikie wrote:

Looks alright to me. (bit awkward having a documented cc1 option - since the cc1 options aren't meant to be user facing - hadn't really thought about/noticed that when reviewing the documentation originally in D95911)

Oh, true. Since we're making ctor homing default maybe I should just make them user facing flags.

I have mixed feelings - on the one hand, yeah, we're changing the behavior of -fno-standalone-debug and giving people a tool to undo that change if it's problematic is good (less chance we have to churn through reverting the change, fixing, reapplying, etc, if there's a way to opt-out, at least in the short term). But equally I don't want to proliferate various rather arbitrary buckets of debug info type homing - I really want to encourage people to think of it as a fairly all-or-nothing situation, because I don't think there's a good way to explain/improve/modify/use the feature if it's "whatever is good enough for my use case". Especially now that there's the standalone_debug attribute that can be used to fix particular use cases where a type is needed across a -g0 boundary.

So... eh. Could go either way - your call, really. Though if we make it a full driver option, maybe a caveat that we may remove the functionality in the future (but that'll mean probably having to leave the flag in as a no-op/with some deprecation warning for some time after removing the functionality).

Yep, good point. Agree that it should be all-or-nothing but I feel like there might be other people in situations like in chrome mac/lldb or what @jmorse mentioned, where we use -fno-standalone-debug because it's impractical to use fstandalone-debug, and having a flag to keep the previous behavior is easier than having people make changes downstream.

Anyway, I think I'll leave it as a cc1 flag for now.

clang/docs/UsersManual.rst

 
 .. option:: -fuse-ctor-homing
 
    This optimization is similar to the optimizations that are enabled as part
    of -fno-standalone-debug. Here, Clang only emits type info for a
    non-trivial, non-aggregate C++ class in the modules that contain a
    definition of one of its constructors. This relies on the additional
    assumption that all classes that are not trivially constructible have a
-   non-trivial constructor that is used somewhere.
+   non-trivial constructor that is used somewhere. The negation,
+   -fno-use-ctor-homing, ensures that constructor homing is not used.
 
    This flag is not enabled by default, and needs to be used with -cc1 or
    -Xclang.
 
 .. option:: -g
 
   Generate complete debug info.
 

clang/include/clang/Driver/Options.td

 def mrelocation_model : Separate<["-"], "mrelocation-model">,
   HelpText<"The relocation model to use">, Values<"static,pic,ropi,rwpi,ropi-rwpi,dynamic-no-pic">,
   NormalizedValuesScope<"llvm::Reloc">,
   NormalizedValues<["Static", "PIC_", "ROPI", "RWPI", "ROPI_RWPI", "DynamicNoPIC"]>,
   MarshallingInfoEnum<CodeGenOpts<"RelocationModel">, "PIC_">;
 def fno_math_builtin : Flag<["-"], "fno-math-builtin">,
   HelpText<"Disable implicit builtin knowledge of math functions">,
   MarshallingInfoFlag<LangOpts<"NoMathBuiltin">>;
+def fno_use_ctor_homing: Flag<["-"], "fno-use-ctor-homing">,
+    HelpText<"Don't use constructor homing for debug info">;
 def fuse_ctor_homing: Flag<["-"], "fuse-ctor-homing">,
     HelpText<"Use constructor homing if we are using limited debug info already">;
 }
 
 def disable_llvm_verifier : Flag<["-"], "disable-llvm-verifier">,
   HelpText<"Don't run the LLVM IR verifier pass">,
   MarshallingInfoNegativeFlag<CodeGenOpts<"VerifyModule">>;
 def disable_llvm_passes : Flag<["-"], "disable-llvm-passes">,

clang/lib/Frontend/CompilerInvocation.cpp

case codegenoptions::NoDebugInfo: // default value
     break;
   case codegenoptions::LocTrackingOnly: // implied value
     DebugInfoVal = None;
     break;
   }
   if (DebugInfoVal)
     GenerateArg(Args, OPT_debug_info_kind_EQ, *DebugInfoVal, SA);
 
-  if (Opts.DebugInfo == codegenoptions::DebugInfoConstructor)
-    GenerateArg(Args, OPT_fuse_ctor_homing, SA);
-
   for (const auto &Prefix : Opts.DebugPrefixMap)
     GenerateArg(Args, OPT_fdebug_prefix_map_EQ,
                 Prefix.first + "=" + Prefix.second, SA);
 
   for (const auto &Prefix : Opts.CoveragePrefixMap)
     GenerateArg(Args, OPT_fcoverage_prefix_map_EQ,
                 Prefix.first + "=" + Prefix.second, SA);
 
if (Arg *A = Args.getLastArg(OPT_debug_info_kind_EQ)) {
     if (Val == ~0U)
       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args)
                                                 << A->getValue();
     else
       Opts.setDebugInfo(static_cast<codegenoptions::DebugInfoKind>(Val));
   }
 
   // If -fuse-ctor-homing is set and limited debug info is already on, then use
-  // constructor homing.
-  if (Args.getLastArg(OPT_fuse_ctor_homing))
-    if (Opts.getDebugInfo() == codegenoptions::LimitedDebugInfo)
+  // constructor homing, and vice versa for -fno-use-ctor-homing.
+  if (const Arg *A =
+          Args.getLastArg(OPT_fuse_ctor_homing, OPT_fno_use_ctor_homing)) {
+    if (A->getOption().matches(OPT_fuse_ctor_homing) &&
+        Opts.getDebugInfo() == codegenoptions::LimitedDebugInfo)
       Opts.setDebugInfo(codegenoptions::DebugInfoConstructor);
+    if (A->getOption().matches(OPT_fno_use_ctor_homing) &&
+        Opts.getDebugInfo() == codegenoptions::DebugInfoConstructor)
+      Opts.setDebugInfo(codegenoptions::LimitedDebugInfo);
+  }
 
   for (const auto &Arg : Args.getAllArgValues(OPT_fdebug_prefix_map_EQ)) {
     auto Split = StringRef(Arg).split('=');
     Opts.DebugPrefixMap.insert(
         {std::string(Split.first), std::string(Split.second)});
   }
 
   for (const auto &Arg : Args.getAllArgValues(OPT_fcoverage_prefix_map_EQ)) {

clang/test/CodeGenCXX/debug-info-ctor-homing-flag.cpp

 // RUN: %clang -cc1 -debug-info-kind=constructor -emit-llvm %s -o - \
 // RUN:        | FileCheck %s -check-prefix=CTOR_HOMING
 // RUN: %clang -cc1 -debug-info-kind=limited -fuse-ctor-homing -emit-llvm %s -o - \
 // RUN:        | FileCheck %s -check-prefix=CTOR_HOMING
 // RUN: %clang -cc1 -debug-info-kind=standalone -fuse-ctor-homing -emit-llvm %s -o - \
 // RUN:        | FileCheck %s -check-prefix=FULL_DEBUG
 // RUN: %clang -cc1 -debug-info-kind=line-tables-only -fuse-ctor-homing -emit-llvm %s -o - \
 // RUN:        | FileCheck %s -check-prefix=NO_DEBUG
 // RUN: %clang -cc1 -fuse-ctor-homing -emit-llvm %s -o - \
 // RUN:        | FileCheck %s -check-prefix=NO_DEBUG
+//
+// RUN: %clang -cc1 -debug-info-kind=constructor -fno-use-ctor-homing \
+// RUN:        -emit-llvm %s -o - | FileCheck %s -check-prefix=FULL_DEBUG
 
 // This tests that the -fuse-ctor-homing is only used if limited debug info would have
 // been used otherwise.
 
 // CTOR_HOMING: !DICompositeType(tag: DW_TAG_structure_type, name: "A"{{.*}}flags: DIFlagFwdDecl
 // FULL_DEBUG: !DICompositeType(tag: DW_TAG_structure_type, name: "A"{{.*}}DIFlagTypePassByValue
 // NO_DEBUG-NOT: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
 struct A {
   A();
 } TestA;