• Extract an NFCI part into D124388

Unfortunatelly, I do not have access to SPEC tests. I checked with 7zip-benchmark and found that Compressing speed is slightly improved with this patch while Decompressing is slightly degraded. But I have to note that compared to a Release build, PGO one is worse even without the patch.

• Update the comment.
• Remove checking for !UnrollPeelMaxCount because a similar condition is anyway checked in line 397.

Our test was for AArch64, so there should be no effects like loop alignment, if I understand you right. I couldn't find a benchmark to test the change against, so I prepared an artificial one based on LLVM test suite/SingleSource/Benchmarks/Adobe-C++/loop-unroll.cpp. I saw the performance gain 10%-30% depending on the iterations count (on x86_64). But I understand that that test is probably biased, so if you recommend some commonly recognized public benchmark that might be suitable for the case, I would try it. As for a more deep analysis, could you share some examples so I can grab some ideas from them?

In D123865#3454386, @davidxl wrote:

For loops with trip count between maxPeelCount and 2*maxPeelCount, this patch enables partial peeling of the loop for maxPeelCount iterations. This feels very aggressive and can be detrimental to performance: 1) increased icache footprint; 2) more pressure to BPU and 3) the back branch of the remaining loop may become less predictable.

What is the motivation case for such a change?

A bit of the context. After D71990, we noticed that one of our tests became slower and did not benefit from PGO. The test has a loop that is executed exactly 8 times, and, before the patch, it was (partially) unrolled but after the patch, the loop is preserved in the compact form.

LGTM

LGTM then

If I understand it right, --fortran-common is useful in rare scenarios but can slow down linking when handling COMMON symbols, and it also hinders implementing the parallel symbol resolution. If that is true, I agree with the change.

I like the idea and don't see real drawbacks, so LGTM.

In D121996#3392197, @nigelp-xmos wrote:

The code LGTM. When object-emission was reintroduced in D98508, there was some discussion on whether XCore ought to use this feature, or provide an integrated assembler instead. Sorry I don't know NVPTX: is NVPTX different because it generates assembly as its natural output, and binary does not make sense? Or could it have an integrated assembler at some point?

LGTM

• Removed llvm::
• Update the test

• Fixed a formatting issue
• Updated the test

LGTM

Thanks for the update! LGTM.

LGTM.

In D119909#3336805, @MaskRay wrote:

Replace getSpecificAllocSingleton<SymbolUnion>().Allocate(firstGlobal); with localSymStorage.

1.02x as fast when linking clang
1.04x as fast when linking chrome

I plan to commit the change separately to decrease diff, but for single threading there is no observable difference for performance.

It looks like the patch was somehow intermixed with D119909. git cannot apply it on ToT.

Hi @beanz! The right tag should be default_triple, not _target. Right now, all these tests are effectively disabled.

LGTM

No objections from me too.

LGTM. Thanks!

Thanks for the suggestions!

Hi @peter.smith! The tests arm-combined-dynrel.s and arm-combined-dynrel-ifunc.s do not run because of inaccurate REQUIRES. Could you fix them, please?

The patch aims to improve the compatibility, right.

The change looks safe. If it eventually helps us to move towards a parallel reading of input files, I am in favor of it.

Changes in LinkerScript.cpp and discard-section-err.s should be committed separately.

• Add comments
• Dump only required information in the test

• Add .o in the test
• Swap then and else branches in InputSection::relocateNonAlloc()

LGTM

LGTM

The output of --why-extract looks useful indeed, but does the map file without output sections make much sense?

LGTM

In D116108#3205038, @dblaikie wrote:

Looks like an acceptable incremental improvement, but perhaps it could be improved further if the generator be setup in the SetUp function and failing that function could implicitly skip tests covered by it, and similar for other tests/setup operations like initializing the MachineRegisterInfo, etc?

In D116107#3205043, @dblaikie wrote:

Is this accurate? Do all uses of DWARFUtils need the asm emission? I'd have thought the DWARFGenerator would mostly be used to go straight to machine code, without assembly?

LGTM