melver wrote:

> > Could you also add the alloc token metadata, and/or whatever else is needed 
> > to cause the allocator calls to kick in, to at least one of the operator 
> > new calls in llvm/test/Transforms/InstCombine/simplify-libcalls-new.ll (or 
> > create a new small test that has this), and invoke various pipelines on it 
> > via opt to ensure we still get the expected transformations (to hot cold 
> > operator new and subsequently the alloc token version)? E.g. `opt 
> > -passes='thinlto<O2>'` and `opt -passes='lto<O2>'` ?
> 
> Largely lgtm to me, except I had a comment about simplifying the cl::opt 
> checks.

Done.

> Also, please add a change to an existing memprof test like suggested above - 
> this will ensure the phase ordering stays correct.

Sorry, missed that.
Added 2 new tests - it seems that we need -supports-hot-cold-new and the index 
so the LTO pipeline doesn't remove the hot/cold hints, correct? I added a 
dedicated test to the AllocToken suite and another in the LTO/X86 suite, the 
latter of which should ensure we catch any deeper regressions.

PTAL.

https://github.com/llvm/llvm-project/pull/169358
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to