[KeyInstr][Clang][NFC] Don't set -dwarf-use-key-instructions

[OCHyams]

Once #144104 lands the flag is true by default (because each DISubprogram will track whether or not it's using key instructions).

[llvmbot]

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

Once #144104 lands the flag is true by default (because each DISubprogram will track whether or not it's using key instructions).

---

Full diff: https://github.com/llvm/llvm-project/pull/144115.diff

2 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Clang.cpp (+1-4)
• (modified) clang/test/DebugInfo/KeyInstructions/flag.cpp (+8-2)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 8556bcadf0915..e3e79a4894cd7 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4627,11 +4627,8 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
   }
 
   if (Args.hasFlag(options::OPT_gkey_instructions,
-                   options::OPT_gno_key_instructions, false)) {
+                   options::OPT_gno_key_instructions, false))
     CmdArgs.push_back("-gkey-instructions");
-    CmdArgs.push_back("-mllvm");
-    CmdArgs.push_back("-dwarf-use-key-instructions");
-  }
 
   if (EmitCodeView) {
     CmdArgs.push_back("-gcodeview");
diff --git a/clang/test/DebugInfo/KeyInstructions/flag.cpp b/clang/test/DebugInfo/KeyInstructions/flag.cpp
index 93503dd4bdb4c..8ce140a2ca031 100644
--- a/clang/test/DebugInfo/KeyInstructions/flag.cpp
+++ b/clang/test/DebugInfo/KeyInstructions/flag.cpp
@@ -8,8 +8,14 @@
 // HELP-NOT: key-instructions
 
 // KEY-INSTRUCTIONS: "-gkey-instructions"
-// KEY-INSTRUCTIONS: "-mllvm" "-dwarf-use-key-instructions"
 
 // NO-KEY-INSTRUCTIONS-NOT: key-instructions
 
-//// TODO: Add smoke test once some functionality has been added.
+// RUN %clang %s | FileCheck %s --check-prefix=SMOKETEST-OFF
+void f() {}
+// SMOKETEST-OFF-NOT: keyInstructions
+// SMOKETEST-OFF-NOT: atomGroup
+
+// RUN %clang %s -gkey-instructions | FileCheck %s --check-prefix=SMOKETEST-ON
+// SMOKETEST-ON: keyInstructions: true
+// SMOKETEST-ON: atomGroup: 1

[jmorse]

Not an issue with this patch but, with no positive HELP: prefixes, this check passes vacuously.

[OCHyams]

I'm not sure that's right - if I change key-instructions to clang I get:

/home/och/dev/llvm-project/clang/test/DebugInfo/KeyInstructions/flag.cpp:8:14: error: HELP-NOT: excluded string found in input
// HELP-NOT: clang
             ^
<stdin>:1:11: note: found here
OVERVIEW: clang LLVM compiler

(Either way - I've noticed that my RUN lines were not running, which I've now fixed)

[jmorse]

Is -gkey-instructions turning -g on as well; would it not be better to explicitly pass -g in both cases anyway?

[OCHyams]

It turns out it is - same happens for gomit-unreferenced-methods so I expect it's a feature of using the g prefix? This was not my original intention, though I don't think it's necessarily a bad thing. Possibly slightly confusing that -gno-key-instructions turns on -g too, though it does make sense if you read it as -g, no key instructions please (again, same as g[no-]omit-unreferenced-methods).

Anyway,

would it not be better to explicitly pass -g in both cases anyway?

Done (-debug-info-kind=line-tables-only).

[OCHyams]

Addressed feedback / replied inline

[OCHyams]

I'm not sure that's right - if I change key-instructions to clang I get:

/home/och/dev/llvm-project/clang/test/DebugInfo/KeyInstructions/flag.cpp:8:14: error: HELP-NOT: excluded string found in input
// HELP-NOT: clang
             ^
<stdin>:1:11: note: found here
OVERVIEW: clang LLVM compiler

(Either way - I've noticed that my RUN lines were not running, which I've now fixed)

[OCHyams]

It turns out it is - same happens for gomit-unreferenced-methods so I expect it's a feature of using the g prefix? This was not my original intention, though I don't think it's necessarily a bad thing. Possibly slightly confusing that -gno-key-instructions turns on -g too, though it does make sense if you read it as -g, no key instructions please (again, same as g[no-]omit-unreferenced-methods).

Anyway,

would it not be better to explicitly pass -g in both cases anyway?

Done (-debug-info-kind=line-tables-only).

[jmorse]

LGTM (my least favourite part of github, aside from it not being Phabricator, is that it chucks away all the inline comments randomly. Yay).

Keeping on the HELP checklines though: if you symlink clang to /bin/true then with no checklines looking for a positive match, the test will still pass, which sucks. (This isn't related to this patches change in behaviour, so the patch can still land).

[OCHyams]

Keeping on the HELP checklines though: if you symlink clang to /bin/true then with no checklines looking for a positive match, the test will still pass, which sucks

Oh I see, I'll add a note to my TODOs to follow up