[PATCH] D15306: [llvm-profdata] Add support for weighted merge of profile data (2nd try)

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 10 15:13:59 PST 2015


silvas requested changes to this revision.
silvas added a reviewer: silvas.
silvas added a comment.
This revision now requires changes to proceed.

In http://reviews.llvm.org/D15306#307232, @slingn wrote:

> From the end user's perspective the <filename>:<weight> syntax seems the most intuitive and concise. And no matter what delimiter we choose (even with a separate option) there will still be edge cases for us to differentiate between file names containing ',' and the user trying but failing to specify a weight.


I disagree. With `<filename>:<weight>` the way to robustly invoke the tool programmatically is non-obvious; for example, the presence of random other files will change our interpretation and cause an error. A tool like this should only look at the files it is told to look at, and no more; with `<filename>:<weight>` we cannot even begin to approach that standard since it is ambiguous which files we should even look at.

With an explicit option, it is something like `'--weighted-file=' + str(weight) + ',' + filename` which always works as expected (when the weight and filename are valid, which is the case I'm talking about).

Also, note that nothing about this is "intuitive" (unless you know of analogous tools that use that particular syntax, but I'm not aware of any). Intuition comes from past experience. People will learn about this feature of llvm-profdata from our documentation; it isn't trying to be compatible with anything and user's don't have any preconceived notions about how it should work. Actually, I would go so far as to say that your statement is backwards: using an option *is* intuitive because users already are already aware of the caveats of having a file whose name might be ambiguous (e.g. starts with `-`) and are aware of the notion of passing options to a command-line program.

Or to put it another way: we should not invent a new command line syntax. Options are the standard way to pass more than a filename.

Furthermore, using an option, we can always go back and add `<filename>:<weight>` if there is resounding user demand for a "more concise" syntax.


http://reviews.llvm.org/D15306





More information about the llvm-commits mailing list