Fix comments.

• Bug fixes
• clang format

Main changes:

clang-formatted.

Updated to top of trunk, bug fixes.

Add new tests for the relaxation code.

clang-format, comments, minor fixes suggested.

Updated patch to address the comments.

Updated test.

Updated with more comments and nits fixed.

Add check for nokeep-text-section-prefix

Add back the test that was omitted in the last update.

New option -z keep-text-section-prefix.

In D45841#1072734, @pcc wrote:

In D45841#1072731, @tmsriram wrote:

In D45841#1072712, @pcc wrote:

Note that this ordering isn't necessarily best for every architecture. On ARM for example it is best to put the hot sections in the middle (see also D44969).

When you say "this ordering" I am confused because currently there is no ordering of the output sections with prefix ".text". Looking at it, seems like the order in which they were seen is the order in the final binary.

Yes, I meant the ordering imposed by this patch which would be hot followed by unlikely, startup, etc.

In D45841#1072712, @pcc wrote:

Note that this ordering isn't necessarily best for every architecture. On ARM for example it is best to put the hot sections in the middle (see also D44969).

Simplify test and condition.

Ping.

Simplify patch to get Module from F.getParent()

Ping.

In D45180#1055227, @espindola wrote:

Thinking a bit more about this:

From the user perspective, the files compiled with -fno-plt should not be responsible for producing a plt. Currently we cannot represent that in the IR. To do that something like RtLibUseGOT would have to be a function attribute on the *caller*. When llvm would decide to call a runtime function to implement llvm.foo, it would check the function using llvm.foo to find out if it should use a got or a plt.

A very similar issue exists for dso_local. If a file is compiled with -fPIC and another without, a function from one but not the other could make dso_local calls to the runtime.

Using these functions attributes would allow a direct mapping of what happens in the non lto case, but would be annoying to work with (what should the inliner do?).

And that is just for the cases where -fno-plt is used in the command line. With a no-plt attribute it could just disappear . Assume we have a no-plt attribute in C and someone adds it to memcpy in a translation unit. An llvm optimization can remove a memcpy that ends up being recreated during codgen and the attribute is lost. There is no good way of avoiding this.

In D45180#1055123, @espindola wrote:

In D45180#1055103, @tmsriram wrote:

In D45180#1055100, @espindola wrote:

I am not sure I understand. Shouldn't this just copy nonlazybind from printf to puts?

I didn't see that happen anywhere, inferLibFuncAttribues is where the attributes are added to the simplified function.

So maybe this is the wrong place to make the change?

Given that we have nonlazybind on each GV, if we are replacing the printf GV with the puts GV we should copy nonlazybind from one to the other.

In D45180#1055100, @espindola wrote:

I am not sure I understand. Shouldn't this just copy nonlazybind from printf to puts?

Ping.

Ping. This patch goes with https://reviews.llvm.org/D42216, Rafael can you approve this too if you think it is ok? Thanks.

Updated patch.

Patch updated.

Updated patch.

Updated to incorporate Rafael's changes.

Rafael's change included and test case updated. "Avoid PLT" changed to "RtLibUseGOT".

METADATA tag changed to "RtLibUseGOT". Also see: https://reviews.llvm.org/D42224/

More information here. This breaks non-x86 builds like PPC because:

Fix comment as suggested.

Change shouldAssumeDSOLocal to be aware of nonlazybind attribute.

Ping. How do we take this forward?

In D39079#905519, @joerg wrote:

In D39079#905468, @rnk wrote:

In D39079#905454, @joerg wrote:

It also increases the pressure on the branch predictor, so it is not really black and white.

I don't understand this objection. I'm assuming that the PLT stub is an indirect jump through the PLTGOT,
not a hotpatched stub that jumps directly to the definition chosen by the loader. This is the ELF model
that I'm familiar with, especially since calls to code more than 2GB away generally need to be indirect anyway.

Correct, so all local calls to the same function go via the same location and share the predication of the indirect jump.

In D39079#905468, @rnk wrote:

In D39079#905454, @joerg wrote:

It also increases the pressure on the branch predictor, so it is not really black and white.

I don't understand this objection. I'm assuming that the PLT stub is an indirect jump through the PLTGOT, not a hotpatched stub that jumps directly to the definition chosen by the loader. This is the ELF model that I'm familiar with, especially since calls to code more than 2GB away generally need to be indirect anyway.

In D39079#905454, @joerg wrote:

In D39079#905396, @rnk wrote:

In D39079#905372, @joerg wrote:

Why again is this a good idea?

It saves the direct jump to the PLT, reducing icache pressure, which is a major cost in some workloads.

It also increases the pressure on the branch predictor, so it is not really black and white.

In D39079#905423, @rnk wrote:

In D39079#905395, @joerg wrote:

Let me phrase it differently. What is this patch (and the matching backend PR) supposed to achieve? There are effectively two ways to get rid of PLT entries:
(1) Bind references locally. This is effectively what -Bsymbolic does and what is breaking the ELF interposition rules.
(2) Do an indirect call via the GOT. Requires knowing what an external symbol is, making it non-attractive for anything but LTO, since it will create performance issues for all non-local accesses (i.e. anything private).

This patch does 2. According to @tmsriram, clever linkers can turn the indirect call back into a nop+call_pcrel32. If this isn't universal, the user must know what their linker supports. I don't see how it causes performance issues for non-local calls, since the PLT will do a jump through the GOT anyway.

In D39079#905372, @joerg wrote:

Why again is this a good idea? This is an even worse hack than -Bsymbolic, the latter at least is visible in ELF header without code inspection. This is breaking core premises of ELF.

In D39079#905353, @hfinkel wrote:

Noting that, as @vit9696 pointed out in D38554, this does not suppress uses of the PLT that occur from backend/optimizer-generated functions (e.g., calls into compiler-rt and similar).

Added test test/CodeGen/noplt.c

Ping.

Ping.

In D30759#696138, @efriedma wrote:

> In theory, the C/C++ standards require behavior equivalent to -fno-merge-all-constants. In practice, code doesn't actually depend on that, so we made the decision many years ago to turn on -fmerge-all-constants by default.

Understood. Does it seem reasonable/useful to fix this along the
lines of GCC, -fmerge-constants and -fmerge-all-constants where the
latter applies to const arrays and a warning that this is happening
when the latter option is used?

-fmerge-all-constants has exactly the same meaning in clang and gcc. And it's generally beneficial for both codesize and performance, so turning it off to pursue performance is a bad idea.

I would suggest finding some other approach to solve your issue later in the optimization pipeline, preferably in a manner which is sensitive to register pressure. Maybe put the code in ConstantHoisting? You don't lose any useful information by promoting the alloca to a global constant; you can easily recreate it

• Fix Help text.
• Fix indentation.

• Changed help text to "Avail copy relocations support for PIE builds"
• Removed mno-pie... as a CC1 option
• Removed help text from mno-pie..

• Option name changed to -mpie-copy-relocations
• Option sets MCPIECopyRelocations in MCOptions