[PATCH] D17555: [Feedback requested] Implement cold spliting

[Xinliang David Li via llvm-commits]

Regarding .text.cold vs .text.unlikely, I am fine either way as long
as they are kept consistent.

Regarding performance impact, Teresa has done extensive tuning in the
past. Our experience is that with PGO, the compiler does a pretty good
job laying out hot BBs in long chains, so the function splitting's
impact on icache is not that significant. With huge page text, the
impact on TLB misses is also moderate.  We do see some improvements
though.

thanks,

David

On Tue, Feb 23, 2016 at 11:13 PM, Amaury SECHET
<deadalnix+llvmreview at gmail.com> wrote:
> deadalnix added a comment.
>
> @joker.eph , this is beneficial for performance for application that are icache and iTLB bound. This works is based on the various patches that were made to use LLVM as a backend for HHVM and was presented here : https://www.youtube.com/watch?v=VZ7A7t5LcR8 .
>
> The optimization is disabled in the general case as it can have negative impact when you aren't icache/itlb bound (for instance bzip).
>
> It may be worthwhile for instrumented build like ASAN and for one shot apps, but I haven't tested this, so don't quote me on this.
>
>
> ================
> Comment at: lib/CodeGen/AsmPrinter/DwarfCFIException.cpp:147
> @@ -147,3 +146,3 @@
>
> -  // Provide LSDA information.
> -  if (!shouldEmitLSDA)
> +void DwarfCFIException::beginFragment(const MachineBasicBlock *MBB,
> +                                      ExceptionSymbolProvider ESP) {
> ----------------
> davidxl wrote:
>> Another candidate of NFC refactoring.
> No, this need to be extracted as this is now needed twice: one for the regular fragment and once for the cold fragment.
>
> ================
> Comment at: lib/MC/MCObjectFileInfo.cpp:448
> @@ -443,1 +447,3 @@
>
> +  ColdTextSection = Ctx->getELFSection(".text.cold", ELF::SHT_PROGBITS,
> +                                       ELF::SHF_EXECINSTR | ELF::SHF_ALLOC);
> ----------------
> davidxl wrote:
>> use .text.unlikely to be consistent with the name used in function reordering.
> cold is the term used all over the place so far. It looks like GCC's crowd want to kill .text.unlikely on their side, so I'd advocate to keep it consistent and go for .cold , unless there is a good reason to stick with .unlikely ?
>
>
> http://reviews.llvm.org/D17555
>
>
>