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

[Mehdi Amini via llvm-commits]

> 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