[llvm] r252563 - [PGO] Make indexed value profile data more compact

[Xinliang David Li via llvm-commits]

Cool. I will investigate that possibility.

David


On Thu, Jan 21, 2016 at 11:49 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> > On Jan 21, 2016, at 11:46 AM, Xinliang David Li <davidxl at google.com>
> wrote:
> >
> > On Thu, Jan 21, 2016 at 11:41 AM, Mehdi Amini <mehdi.amini at apple.com>
> wrote:
> >>
> >> On Jan 21, 2016, at 11:32 AM, Xinliang David Li via llvm-commits
> >> <llvm-commits at lists.llvm.org> wrote:
> >>
> >>
> >>
> >> On Thu, Jan 21, 2016 at 11:26 AM, Vedant Kumar via llvm-commits
> >> <llvm-commits at lists.llvm.org> wrote:
> >>>
> >>> I haven't seen a failure yet, this is a latent issue.
> >>>
> >>> AFAICT mutable static variables are generally avoided in llvm. We use
> >>> llvm::ManagedStatic with atomics or locking if absolutely necessary.
> >>>
> >>> Could you move this field into InstrProfWriter and
> InstrProfRecordTraits?
> >>
> >>
> >> The problem is that the trait's EmitData method (which references the
> >> variable) is a static method as well ..
> >>
> >>
> >> Naive comment/question: if it is just a debug/testing flag, why not
> making
> >> it a cl::opt one?
> >
> > yes -- the interface is used by unittest. Not sure if cl::opt can work
> > in this case?
>
> The unit tests would have to call ParseCommandLineOptions, there is a
> precedent for that, see  unittests/Analysis/MixedTBAATest.cpp
> (I’m not saying this is *the* thing to do in your case, just pointing the
> option).
>
> —
> Mehdi
>
>
> >
> > David
> >
> >>
> >> —
> >> Mehdi
> >>
> >>
> >>
> >> David
> >>
> >>>
> >>>
> >>> vedant
> >>>
> >>>> On Jan 21, 2016, at 9:58 AM, Xinliang David Li <davidxl at google.com>
> >>>> wrote:
> >>>>
> >>>> The setter call is invoked only in a unittest. Can you send me the
> >>>> failure report? (currently InstrProfRecordTraits has no members and is
> >>>> stateless).
> >>>>
> >>>> thanks,
> >>>>
> >>>> David
> >>>>
> >>>> On Thu, Jan 21, 2016 at 1:17 AM, Vedant Kumar <vsk at apple.com> wrote:
> >>>>> Hi David,
> >>>>>
> >>>>>> namespace {
> >>>>>> +static support::endianness ValueProfDataEndianness =
> support::little;
> >>>>>> +// Internal interface for testing purpose only.
> >>>>>> +void InstrProfWriter::setValueProfDataEndianness(
> >>>>>> +    support::endianness Endianness) {
> >>>>>> +  ValueProfDataEndianness = Endianness;
> >>>>>> +}
> >>>>>
> >>>>>
> >>>>> Please remove this static variable. It breaks clients who use llvm
> in a
> >>>>> multi-threaded environment.
> >>>>>
> >>>>> thanks
> >>>>> vedant
> >>>>>
> >>>
> >>> _______________________________________________
> >>> llvm-commits mailing list
> >>> llvm-commits at lists.llvm.org
> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >>
> >>
> >> _______________________________________________
> >> 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/20160121/f7fac3b7/attachment.html>