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

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 21 11:41:50 PST 2016


> 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 <mailto: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?

— 
Mehdi



> David
>  
> 
> vedant
> 
> > On Jan 21, 2016, at 9:58 AM, Xinliang David Li <davidxl at google.com <mailto: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 <mailto: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 <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <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/738bdf4c/attachment-0001.html>


More information about the llvm-commits mailing list