[PATCH] D135127: [llvm-profdata] Add --output-format option

Ellis Hoag via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 5 09:14:23 PDT 2022


ellis added a comment.

In D135127#3835933 <https://reviews.llvm.org/D135127#3835933>, @davidxl wrote:

> 2. what is the use case of splitting text and text-encoding?

The existing `--text` option for the show command is used to emit the profile in the text-encoded format. I thought it would be confusing use `--output-format=text` to mean this, since I would expect this to mean "print normal text output". So I decided on the following flags:

- `--output-format=text`
  - The show command will print text that is useful to the user
- `--output-format=text-encoding`
  - Will emit the profile in the text-encoded format without additional output. This is identical to the `--text` flag.

I know this is a bit weird. So I'm wondering if it's better to leave the `--text` flag alone and only have `--output-format=<json|yaml>`. Let me know what you think!



================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:60
 
+enum class ShowOutputFormat { Text, TextEncoding, Json, Yaml };
+
----------------
MaskRay wrote:
> Should this be `OutputFormat` instead? Then you can use a different name for the variable...
I thought the name `OutputFormat` would be confusing because one might think it had the meaning of `ProfileFormat` above. `ProfileFormat` specifies which profile format we should output, while `ShowOutputFormat` only specifies the output for the show command. We don't currently have YAML or JSON profile formats.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135127/new/

https://reviews.llvm.org/D135127



More information about the llvm-commits mailing list