[all-commits] [llvm/llvm-project] 627692: [ThinLTO] Mark callers of local ifunc not eligible...

[Fangrui Song via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 6276927bf3f6ce4a9ef0b9941b2c6450ae4cd1eb
      https://github.com/llvm/llvm-project/commit/6276927bf3f6ce4a9ef0b9941b2c6450ae4cd1eb
  Author: Fangrui Song <i at maskray.me>
  Date:   2023-08-29 (Tue, 29 Aug 2023)

  Changed paths:
    M llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
    A llvm/test/ThinLTO/X86/ifunc-import.ll

  Log Message:
  -----------
  [ThinLTO] Mark callers of local ifunc not eligible for import

Fix https://github.com/llvm/llvm-project/issues/58740
The `target_clones` attribute results in ifunc on eligible targets
(Linux glibc/Android or FreeBSD). If the function has internal linkage,
we will get an internal linkage ifunc.
```
__attribute__((target_clones("popcnt", "default")))
static int foo(int n) { return __builtin_popcount(n); }
int use(int n) { return foo(n); }

@foo.ifunc = internal ifunc i32 (i32), ptr @foo.resolver
define internal nonnull ptr @foo.resolver() comdat {
; local linkage comdat is another issue that should be fixed
  ...
  select i1 %.not, ptr @foo.default.1, ptr @foo.popcnt.0
  ...
}
define internal i32 @foo.default.1(i32 noundef %n)
```

ifuncs are not included in module summaries, so LTO doesn't know the
local linkage `foo.default.1` referenced by `foo.resolver`
should be promoted. If a caller of `foo` (e.g. `use`) is imported,
the local linkage `foo.resolver` will be cloned as a definition
(IRLinker::shouldLink), leading to linker errors.
```
ld.lld: error: undefined hidden symbol: foo.default.1.llvm.8017227050314953235
>>> referenced by bar.c
>>>               lto.tmp:(foo.ifunc)
```

As a simple fix, just mark `use` as not eligible for import. Non-local
linkage ifuncs do not have the problem, because they are not imported,
and not cloned when a caller is imported.

---

https://reviews.llvm.org/D82745 contains a more involved fix, though the
original bug it intended to fix
(https://github.com/llvm/llvm-project/issues/45833) now works.

Note: importing ifunc is tricky.
If we import an ifunc, we need to make sure the resolver and the
implementation are in the translation unit, as required by
https://sourceware.org/glibc/wiki/GNU_IFUNC

> Requirement (a): Resolver must be defined in the same translation unit as the implementations.

This is infeasible if the implementation is changed to
available_externally.

In addition, the imported ifunc may be referenced by two translation
units. This doesn't work with PowerPC32 -msecure-plt
(https://maskray.me/blog/2021-01-18-gnu-indirect-function).
At the very least, every referencing translation unit needs one extra
IRELATIVE dynamic relocation.

At least for the local linkage ifunc case, it doesn't have much use
outside of `target_clones`, as a global pointer is usually a better
replacement.

I think ifuncs just have too many pitfalls to design more IR features
around it to optimize them.

Reviewed By: tejohnson

Differential Revision: https://reviews.llvm.org/D158961