[PATCH] D14547: [llvm-profdata] Add support for weighted merge of profile data

[Sean Silva via llvm-commits]

On Wed, Nov 11, 2015 at 1:50 PM, Xinliang David Li via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> We can simply document it. In other words, user is not expected  to use
> ":<num>" or ",<num>" as file suffix for profile data.
>

And if they would like to be bulletproof, then they should manually append
`:1` or `:<num>`.

E.g. suppose they have a file called $STR. If they call `llvm-profdata
merge $STR:1` then they are guaranteed that $STR will be interpreted
properly no matter what characters it contains.

Another possibility is to have a syntax `llvm-profdata merge
--file-with-weight=42,path/to/foo --file-with-weight=17,path/to/bar`. That
way, we don't have any ambiguity in any scenario (except the existing
potential ambiguity with names that start with `-`, but users already have
to deal with this issue). That seems bulletproof, and we can document that
plain `path/to/foo` is just syntax sugar for
`--file-with-weight=1,path/to/foo`.

-- Sean Silva


>
> David
>
> On Wed, Nov 11, 2015 at 1:45 PM, Justin Bogner <mail at justinbogner.com>
> wrote:
>
>> Xinliang David Li <davidxl at google.com> writes:
>> > On Tue, Nov 10, 2015 at 2:36 PM, Diego Novillo <dnovillo at google.com>
>> wrote:
>> >> On Tue, Nov 10, 2015 at 3:07 PM, David Li <davidxl at google.com> wrote:
>> >>> ================
>> >>> Comment at: test/tools/llvm-profdata/weight-sample.test:42
>> >>> @@ +41,3 @@
>> >>> +3- Bad merge: foo and bar profiles with too few weight values
>> >>> +RUN: not llvm-profdata merge --sample --text -weights=2
>> >>> %p/Inputs/weight-sample-bar.proftext
>> %p/Inputs/weight-sample-foo.proftext
>> >>> -o %t.out 2>&1 | FileCheck %s --check-prefix=ERROR1
>> >>> +ERROR1: error: Profile weight must be specified for each input file.
>> >>> ----------------
>> >>> No need for such tests if the weight is always attached to the input
>> file.
>> >>>
>> >>
>> >> But then what happens if one of the files is specified without
>> ":WEIGHT"?
>> >> Should we complain about it?  Should we assume MAX(other weights)?
>> >
>> > The default is 1 of course.
>> >
>> >> I'm not sure I'm completely convinced that file1:W1 file2:W2 is better
>> >> than --weights=W1,W2.  What if ':' is part of the filename?  I know,
>> seems
>> >> like a stretch.  I'm not actually arguing against it.  I mostly find
>> it a
>> >> bit odd.
>> >
>> > : is just a suggestion, you can use coma to separate it.
>> >
>> > pros:
>> >
>> > 1) the weight and file name are always 'together'. Think about a long
>> list
>> > of files -- having a separate list option make it harder to associate
>> and
>> > and  can not really skip the default weight of 1, e.g
>> > -W10,1,1,1,1,1,1,1,1,1,20
>> > 2) there is no need for an additional option
>> > 3) the implementation is simpler and requires fewer code.
>> >
>> > cons:
>> > I don't see any real cons -- except that you find it weird -- but why?
>>
>> The biggest con is probably ambiguity. Using a comma isn't really any
>> better than using a colon here, both are perfectly valid in filenames.
>> I guess it's probably not likely that anyone has a file named foo and a
>> file named foo:10 and wants to use foo with a weight of 10, but the code
>> will certainly have to deal with that case if we're attaching the
>> weights to the filenames.
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151111/f79a15d4/attachment.html>