[llvm-objdump] Add triple support to mcpu=help

[cabbaken]

Current --mcpu=help needs -d to work.
I specialise --mcpu=help, you can just use --mcpu=help --triple without specify -d now.

Fixes: #150567

[llvmbot]

@llvm/pr-subscribers-llvm-binary-utilities

Author: Ruoyu Qiu (cabbaken)

Changes

Current --mcpu=help needs -d to work.
I specialise --mcpu=help, you can just use --mcpu=help --triple without specify -d now.

Fixes: #150567

---

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

2 Files Affected:

• (modified) llvm/test/tools/llvm-objdump/mattr-mcpu-help.test (+6)
• (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+17)

diff --git a/llvm/test/tools/llvm-objdump/mattr-mcpu-help.test b/llvm/test/tools/llvm-objdump/mattr-mcpu-help.test
index 65c426008fd6a..3475f0396d9b2 100644
--- a/llvm/test/tools/llvm-objdump/mattr-mcpu-help.test
+++ b/llvm/test/tools/llvm-objdump/mattr-mcpu-help.test
@@ -14,3 +14,9 @@ FileHeader:
   Data:            ELFDATA2LSB
   Type:            ET_EXEC
   Machine:         EM_X86_64
+
+# RUN: llvm-objdump --triple=x86_64 --mcpu=help 2>&1 \
+# RUN:   | FileCheck %s --check-prefix=CHECK-WITHOUT-DISASSEMBLING
+
+# CHECK-WITHOUT-DISASSEMBLING: Available CPUs for this target:
+# CHECK-WITHOUT-DISASSEMBLING: Available features for this target:
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 3ec644a472bfc..5fe4420eb4f2d 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -3533,6 +3533,19 @@ commaSeparatedValues(const llvm::opt::InputArgList &InputArgs, int ID) {
   return Values;
 }
 
+static int MCPUHelp() {
+  if (!TripleName.empty()) {
+    std::string Error;
+    const Target *DummyTarget = TargetRegistry::lookupTarget(TripleName, Error);
+    if (!DummyTarget) {
+      outs() << Error << '\n';
+      return 2;
+    }
+    DummyTarget->createMCSubtargetInfo(TripleName, MCPU, "");
+  }
+  return 0;
+}
+
 static void parseOtoolOptions(const llvm::opt::InputArgList &InputArgs) {
   MachOOpt = true;
   FullLeadingAddr = true;
@@ -3826,6 +3839,10 @@ int llvm_objdump_main(int argc, char **argv, const llvm::ToolContext &) {
       !DisassembleSymbols.empty())
     Disassemble = true;
 
+  if (!Disassemble && MCPU == "help") {
+    return MCPUHelp();
+  }
+
   if (!ArchiveHeaders && !Disassemble && DwarfDumpType == DIDT_Null &&
       !DynamicRelocations && !FileHeaders && !PrivateHeaders && !RawClangAST &&
       !Relocations && !SectionHeaders && !SectionContents && !SymbolTable &&

[cabbaken]

It appears my local code is currently outdated. I will address this issue and push the fix later.

[cabbaken]

@jh7370
Would it be necessary or more appropriate to create a separate PR to implement the object triple detection logic for --mcpu=help?

[jh7370]

No braces for single-line conditionals.

The return will prevent other llvm-objdump functionality from executing, if --mcpu=help but NOT --disassemble or similar. Previously, if --mcpu=help was specified, the other operations still printed as normal. I think we should retain this behaviour, i.e. don't return here.

[cabbaken]

If we don't return here, llvm-objdump --triple=x86_64 --mcpu=help will eventually reach return 2; later in the code, indicating an abnormal exit.
Would it be better to place the return statement right after the T->printHelp() and and include a check (if mcpu=="help") within the existing long conditional block?
Here's what I mean:

@@ -3840,15 +3842,11 @@ int llvm_objdump_main(int argc, char **argv, const llvm::ToolContext &) {
       !DisassembleSymbols.empty())
     Disassemble = true;
 
-  if (!Disassemble && MCPU == "help") {
-    return MCPUHelp();
-  }
-
   if (!ArchiveHeaders && !Disassemble && DwarfDumpType == DIDT_Null &&
       !DynamicRelocations && !FileHeaders && !PrivateHeaders && !RawClangAST &&
       !Relocations && !SectionHeaders && !SectionContents && !SymbolTable &&
       !DynamicSymbolTable && !UnwindInfo && !FaultMapSection && !Offloading &&
-      !(MachOOpt &&
+      MCPU != "help" && !(MachOOpt && 
         (Bind || DataInCode || ChainedFixups || DyldInfo || DylibId ||
          DylibsUsed || ExportsTrie || FirstPrivateHeader ||
          FunctionStartsType != FunctionStartsMode::None || IndirectSymbols ||
@@ -3858,6 +3856,9 @@ int llvm_objdump_main(int argc, char **argv, const llvm::ToolContext &) {
     return 2;
   }
 
+  if (!Disassemble && MCPU == "help")
+    mcpuHelp();
+
   DisasmSymbolSet.insert_range(DisassembleSymbols);
 
   llvm::for_each(InputFilenames, dumpInput);

[cabbaken]

I am wondering if the check (if (!Disassemble && MCPU == "help")) should be as extensive as the longer conditional block (or put this logic into the body of long conditional check). For example, executing llvm-objdump --triple=x86_64 --mcpu=help --all-headers currently does not provide the list of CPUs and features.

This Pull Request is only intended to provide an easier way for users to employ the --mcpu=help option. Given this goal, is it necessary to check all of the other conditions?

[jh7370]

Name functions properly. This function name fails on both rules listed for functions.

[jh7370]

Why is this writing an error to stdout not stderr. In fact, why is it not simply using the existing mechanism for reporting errors from llvm-objdump?

[jh7370]

It's probably worth a comment here about what is going on.