ibookstein added a comment.
> To me the 'not in the weeds' part is, "How do I get CPU-dispatch/CPU-specific
> to work without RAUW, since that is offensive to the CFE code owner?
> Additionally, I found that some of the examples without the defined resolver
> actually DO work in my downstream, though I don't know what changes we make
> to make that happen. So adding this limitation in actually breaks my
> downstream.
Regarding the examples that do work, I've provided explanations as to how they
partially sort-of-work, but that the general semantics are broken (the
'connection' to the resolver is 'severed' in each translation unit that has it
as a declaration + all references from within that TU are borked).
The problems that the CFE currently uses RAUW to solve are //fundamental// to
the specified behavior of the **language features** that use it, so a solution
for these problems needs to arise from the CFE. Imagine the following
incremental compilation use-case:
> extern int x; // x is a GlobalVariable declaration (initializer == null).
> int y; // y is a GlobalVariable definition (initializer = constant 0).
> extern int x __attribute__((alias("y"))); // x needs to transform into a
GlobalAlias now.
>
> extern void *my_memcpy(void *dest, void *src, size_t n); // my_memcpy is a
Function declaration
> void *my_memcpy(void *dest, void *src, unsigned long n)
__attribute__((ifunc("my_memcpy_resolver"))); // my_memcpy_resolver is a
Function declaration, my_memcpy is a GlobalIFunc with declaration for a
resolver - whoops, incremental module is invalid at this point
> static void *my_memcpy_impl(void *dest, void *src, unsigned long n) {
return 0; }
> static void *my_memcpy_resolver(void) { return &my_memcpy_impl; } // Now
my_memcpy ifunc has defined resolver
>
> void CpuSpecific(void); // CpuSpecific is a Function declaration
> void Foo(void) { CpuSpecific(); } // Foo calls a function declaration
> __attribute__((cpu_specific(generic))) void CpuSpecific(void) {
puts("generic"); } // We still don't know whether cpu_dispatch will be in this
TU or not. Do we upgrade CpuSpecific from a function declaration to a
definition? to an ifunc? Bind directly to this body?
> __attribute__((cpu_dispatch(generic))) void CpuSpecific(void); // Now we
know that we have to upgrade it to an ifunc, and how the resolver should look.
But it's already a Function (declaration).
All the above //need// to work in non-incremental compilation, and the only
existing way for them to work right now is RAUW or a module finalization step.
If the CFE code owner(s) find that offensive but proposes no alternative, what
is one to do? There are 3 possible states here:
1. Remain broken
2. Solve using RAUW/finalization
3. Solve using yet-unproposed non-RAUW solution (which solves all of the above
use-cases simultaneously because they're essentially the same)
It is better to treat (2) as a way out of (1) which doesn't increase the cost
of (3), than to just stay at (1) until (3) happens. As it currently stands, the
somewhat similar issue of calling a declared-but-undefined function in the REPL
is currently embodied as a JIT session error, with failure to materialize
symbols. Perhaps a valid solution for the work-in-progress aliases and ifuncs
is to transform them into declarations in the JIT module until they have
definitions for their aliasees/resolvers. But we won't know without more
context. I think it might be more effective that I'll write a patch up which
does use RAUW together with a test for the breakage we discussed and we'll
continue the discussion with the CFE code owner(s) there.
>> The LLVM bitcode verifier does not consider GlobalAliases which have either
>> a null or an undefined aliasee to be valid. The same should have held for
>> GlobalIFuncs and their resolvers since their inception, and the fact that it
>> were not so is an oversight.
>
> Citation Needed here. Is there a design document that specifies this, or is
> this your opinion? If its the latter, the implementation was the
> documentation, so this is still a breaking change.
I'm assuming that you're talking about GlobalIFuncs, since for GlobalAliases
you'll see both constraints encoded in the `Verifier::visitGlobalAlias` and
`Verifier::visitAliaseeSubExpr` functions.
As for GlobalIFunc, I have no design document for the LLVM-IR-level
representation of the feature, but:
1. @MaskRay has provided reference for the object-format constraint itself
(`Requirement (a): Resolver must be defined in the same translation unit as the
implementations.`). Bitcode Modules correspond to TUs - so it stands to reason
that the same restriction apply to them, unless behavior is implemented to
bridge the gap.
2. I've demonstrated that compiling a bitcode module containing an ifunc with
an undefined resolver emits broken code for all usages of the
ifunc-with-no-resolver, where I can probably massage that into a crash with a
small bit of additional work (since it ends up calling the resolver rather than
calling the return value of the resolver). This applies to current use-cases of
cpu_specific in translation units that don't contain the corresponding
cpu_dispatch, which I understand is part of the documented usage model of
cpu_specific/cpu_dispatch. So it's already subtly broken, and in reference to
(1), this means that behavior to bridge the gap between what the IR ostensibly
allows and what the object format allows is **not** implemented.
3. CFE already diagnoses this issue for plain ifuncs and aliases.
4. I've drawn significant parallels between aliases and ifuncs at the object
file level, and extrapolated the reasoning for why aliases have to point at
defined objects to the reasoning why ifuncs have to point at defined objects.
So, when I say "should have held" I am indeed expressing my opinion. But I do
believe that I've backed it up substantially, and that from a design
perspective adding that restriction to the IR really is the correct decision.
Calling the implementation the documentation doesn't ring right with me - is
the broken behavior I referred to in (2) a compatibility constraint now? Are
the bizarre disallowed-by-the-specification undefined STT_GNU_IFUNC symbols
emitted by cpu_specific-without-cpu_dispatch a compatibility constraint now?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112349/new/
https://reviews.llvm.org/D112349
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
