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

[Xinliang David Li via llvm-commits]

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?

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
>
>