In D72829#1845396, @serge-sans-paille wrote:

I suppose you mean whether "SemanticInterposition" should be invalidated when "PIC Level" does not exist or "PIE Level" exists. I am a bit inclined to make it more flexible/orthogonal in the backend, i.e. SemanticInterposition is in affect even if "PIE Level" is set.

OK, looks good as is, then?

In D73228#1845102, @peter.smith wrote:

To make sure I understand, this is adding a local alias as at assembly time that can be used as a fixup target. This is necessary as at assembly time all we have is a global default visibility symbol which we have to assume can be interposed, even if the code-generator has already assumed it. Are there other uses that I'm missing?

In D73474#1845127, @nickdesaulniers wrote:

@hans can we please cherry pick this into the llvm-10 release, if it's not too late?

It landed as 4f38ab250ff4680375c4c01db0a88c157093c665. 3238b03c197741207dea8cc3bc3273f74b448460 landed before it, but doesn't cherry-pick cleanly, where as I believe that 4f38ab250ff4680375c4c01db0a88c157093c665 does. 3238b03c197741207dea8cc3bc3273f74b448460 is NFC and not required.

Improve comment for AsmPrinter::getSymbolPreferLocal()

In D72829#1844050, @serge-sans-paille wrote:

@MaskRay should we add a verifier step to check that pie/pic/semanticinterposition module flags are consistent, or leave that to clang ?

Improve tests

.

Replace operator[] with

test/CodeGen/X86/catchpad-lifetime.ll (added in rL257158 @majnemer) appears to be bit-rotted. If I delete:

Does this depend on D69868?

Thanks. b0e979724f2679e4e6f5b824144ea89289bd6d56 intended to make newpm and oldpm -print-after-all output similar. I did not realize llvm-dis output is also affected.

Ping D73228 and D73230 (tested on a large code base).

and allows for better

Closed by commit 369ea47b9249f1843730d8d0c0ca5df46826d032

is too recent for current CMake version

It seems that callbr (with output) will now be similar to a catchpad. It can set live-in physical register information. (See test/CodeGen/X86/{seh-catch-all.ll,seh-exception-code.ll,wineh-coreclr.ll,wineh-exceptionpointer.ll}) Is there any caveat doing this? Add @rnk to the attention list as the author of rL249492 and rL249786...

At a minimum, test/Driver/freebsd.c and test/Preprocessor/init-ppc64.c should be updated for powerpcle-unknown-freebsd tests.

Simplify via getPCBias()

Simplify

Simplify

Handle R_PPC_LOCAL24PC as well

Fix PLTresolve comments

Make glibc work

Move canonical PLT entries to the beginning of .glink
Otherwise Symbol::getVA is not stable

Fix st_value

Adopt grimar's suggestion

.

In D73307#1839880, @mtrofin wrote:

In D73307#1839829, @MaskRay wrote:

The code change seems fine, but the test requires some changes. I haven't followed Propeller development, but hope someone with profile experience can confirm InternalLinkage is the only linkage we need to care about (otherwise the option will be a misnomer if we ever extend it) and check whether this feature is useful on its own. Does it improve profile precision?

I can comment on the usefulness aspect: we had an earlier prototype of this, which we tried on a real-world application benchmark. The binary had ~10% of local statics exhibiting duplicate names. Ensuring unique names led to observable differences in the AFDO file (i.e. some of those functions had profiles that, before, were lost for one of the duplicates, and now were correctly attributed to the different functions), and a measurable performance improvement.

The code change seems fine, but the test requires some changes. I haven't followed Propeller development, but hope someone with profile experience can confirm InternalLinkage is the only linkage we need to care about (otherwise the option will be a misnomer if we ever extend it) and check whether this feature is useful on its own. Does it improve profile precision?

In D72831#1839293, @smeenai wrote:

CC @hans ... there was discussion in https://bugs.llvm.org/show_bug.cgi?id=44397 about whether this is worth picking to 10.0.

In D72222#1838988, @mrutland wrote:

In D72222#1837536, @MaskRay wrote:

For BTI c issue, GCC has several releases that do not work with -mbranch-protection=bti. The Linux kernel has to develop some mechanism to detect the undesirable placement of bti c, if there are -mbranch-protection=bti users. So I don't think that inconsistency in clang 10.0.0 with GCC will be a problem.

Speaking as the person implementing the Linux side of things, I think that will be a problem. Kernel-side we want consistency across compilers.

For Linux we were planning to do a very simple build-time test to rule out compilers with the broken behaviour. We would expect functional compilers to have a consistent layout for a given combination of options, and we would consider this layout to be ABI.

The compile time check would compile a trivial test file, e.g.

void function(void) { }

... with flags:

-fpatchable-function-entry=2 -mbranch-protection=bti

... and if the function entry point is a NOP, mark that compiler as broken. In practice, this will mean that the kernel build system will disable BTI support for those broken compilers.

Trying to support different layout approaches within Linux will add a fair amount of unnecessary complexity which we cannot contain in one place, and makes it more likely that support for either case will be broken in future.

For the M=0 case, I would prefer to avoid runtime detection unless really necessary.

For the M!=0 case, which I believe Linux will need to use in the near future, I realise that a runtime check will be necessary to detect the absence of a BTI. Regardless, consistent behaviour across compilers will make this significantly simpler and less error-prone.

Thanks,
Mark.

@hans OK to cherry pick this commit? We would also need d232c215669cb57f5eb4ead40a4a336220dbc429 ([AsmPrinter] Don't emit __patchable_function_entries entry if "patchable-function-entry"="0"; a GCC compatibility issue I did not notice when I implemented -fpatchable-function-entry=N,0).

In D73301#1839138, @nickdesaulniers wrote:

Thanks, with this patch applied, I was able to build an allyesconfig aarch64 linux-next kernel, and the llvm test suite passes. LGTM

Simplify test