Thanks for the responses! I’m going to see if I can summarize the concerns and ideas people have (for my own clarity) and see where we can go on from there. Folks seem to be on board with the idea of introducing some new IR entity that (after linking) could be a reference into the PLT, but some kinks need to be worked out first:
Naming (Thanks for clarifications maskray@. I mixed up some terminology and concepts.): Because the PLT is primarily the concern of the linker, the naming probably shouldn’t be directly tied to “PLT”. The initial proposal was for something that matched the @plt modifier on x86, so that’s what inspired the naming. The intended behavior of this IR level change is that at least on x86 or aarch64, the resolved constant could be lowered to something that has the @plt syntax, but I suppose other targets could have their own meaning for “the address of this function is insignificant.”
Abstraction: The IR representation of this probably shouldn’t be too strictly mapped to object file representations. It’s useful to have an IR pattern that can be mapped to relocations on different binary formats, but we don’t want to introduce a state where we have new Constants for individual relocations. The IR-entity should remain abstract enough that it’s not tied to a specific relocation, but it can still be lowered appropriately by different backends.
As an update to the proposal, instead of pltentry(@func), we can call it something like unnamedfunc(@func) and everywhere it’s used, it means: “The value used here is functionally equivalent to the original function, but may not be a reference to the original function. The address of this value is insignificant.” This is leveraged from unnamed_addr where the address of a global variable is insignificant, but this would instead be tied to instances where the function is used rather than be attached to the function declaration/definition. unnamedfunc(@func) could be lowered to a direct reference (func), the @plt modifier on x86/aarch64 (func@plt), a thunk, or anything that’s equivalent to the resolved function at runtime.
Sorry for the delay responding Leonard. I don’t really understand your rationale here. A PLT entry is a completely target specific concept because some targets don’t have PLTs. I don’t think there is any reason that a frontend would abstractly generate this unless they already have a target-specific plan in mind.
If you go with your “unnamedfunc” approach, you’ll have to define the semantics of what that means, and it will need to mean something on targets without a PLT. If it isn’t generally implementable, then it is target specific again.
I feel like you are trying (earnestly!) to make the IR better here, but by making this abstract it is actually just making it more opaque for no obvious benefit.
Implementation-wise, I imagine we don’t want this as a subclass of GlobalValue. As Peter suggested, this may not eventually lower to a symbol. If it were a GlobalValue, that would also imply linkage types and visibility would also apply to it which might not make sense. A GlobalValue also seems to imply a module-level entity when this would primarily be used on individual locations where a function would normally be used.
I agree, this should be a subclass of ConstantExpr.
-Chris
Thanks for the responses! I’m going to see if I can summarize the concerns and ideas people have (for my own clarity) and see where we can go on from there. Folks seem to be on board with the idea of introducing some new IR entity that (after linking) *could* be a reference into the PLT, but some kinks need to be worked out first:
*Naming* (Thanks for clarifications maskray@. I mixed up some terminology and concepts.): Because the PLT is primarily the concern of the linker, the naming probably shouldn’t be directly tied to “PLT”. The initial proposal was for something that matched the @plt modifier on x86, so that’s what inspired the naming. The intended behavior of this IR level change is that at least on x86 or aarch64, the resolved constant could be lowered to something that has the `@plt` syntax, but I suppose other targets could have their own meaning for “the address of this function is insignificant.”
*Abstraction*: The IR representation of this probably shouldn’t be too strictly mapped to object file representations. It’s useful to have an IR pattern that can be mapped to relocations on different binary formats, but we don’t want to introduce a state where we have new Constants for individual relocations. The IR-entity should remain abstract enough that it’s not tied to a specific relocation, but it can still be lowered appropriately by different backends.
As an update to the proposal, instead of `pltentry(@func)`, we can call it something like `unnamedfunc(@func)` and everywhere it’s used, it means: “The value used here is functionally equivalent to the original function, but may not be a reference to the original function. The address of this value is insignificant.” This is leveraged from `unnamed_addr` where the address of a global variable is insignificant, but this would instead be tied to instances where the function is used rather than be attached to the function declaration/definition. `unnamedfunc(@func)` could be lowered to a direct reference (func), the @plt modifier on x86/aarch64 (func@plt), a thunk, or anything that’s equivalent to the resolved function at runtime.
Sorry for the delay responding Leonard. I don’t really understand your rationale here. A PLT entry is a completely target specific concept because some targets don’t have PLTs. I don’t think there is any reason that a frontend would abstractly generate this unless they already have a target-specific plan in mind.
If you go with your “unnamedfunc” approach, you’ll have to define the semantics of what that means, and it will need to mean something on targets without a PLT. If it isn’t generally implementable, then it is target specific again.
I feel like you are trying (earnestly!) to make the IR better here, but by making this abstract it is actually just making it more opaque for no obvious benefit.
+1 to this. LLVM already has a large issue with implicit ABI contracts between Clang (and other frontends) and the various backends. We should not make that worse. The problem here is that there are multiple ways to represent the reference to the function symbol, and in this case, there's an ABI requirement to pick a specific one of them. We should make that clear and explicit. If there's an abstraction here that's useful, it's in the way to pass along that target-specific information -- I think of this like a target-specific attribute.
-Hal
Completely agreed, a lot of my perspective comes from bitter experience having messed up the ABI lowering design :-). Sorry for that 
-Chris
I think I follow. So in this case, it’s better to be explicit and say “this could lower to a PLT entry (which is only supported on specific targets)” rather than making something general that can exist on all targets. Makes sense. I wasn’t sure if there was perhaps something equivalent on other targets that this could lower to, but we can make this target/PLT specific.
IIUC, the actual requirements for the proposed pltentry(@X) constant is:
- The returned address MUST have a constant offset at link-time from some other unspecified but defined-in-the-same-binary/DSO symbol Y. Which symbol it is is presumed not to matter because all locally-defined symbols have constant offsets from each-other, anyhow.
- The address is otherwise insignificant. (Therefore, coming up with a brand new unique address, by creating a local stub function, would be an acceptable implementation.)
These requirements do seem somewhat generic: even on a system which has a different way to make a call could still create a local stub function, and give you the address of that. However, “unnamed address” isn’t a good name, because it doesn’t capture the first requirement, only the second.
I see. Perhaps something like “dso_local_stub” or “dso_local_unnamed_stub” would be better naming? The dso_local bit I think captures the first requirement and, if kept generic, we could have the default implementation be this local stub.
Hi all. Are there any thoughts on the new name idea (“dso_local_stub” or “dso_local_unnamed_stub”)? I’d like to see if I can move forward with my patch.
IIUC, the actual requirements for the proposed pltentry(@X) constant is:
- The returned address MUST have a constant offset at link-time from
some other unspecified but defined-in-the-same-binary/DSO symbol Y. Which
symbol it is is presumed not to matter because all locally-defined symbols
have constant offsets from each-other, anyhow.
- The address is otherwise insignificant. (Therefore, coming up with a
brand new unique address, by creating a local stub function, would be an
acceptable implementation.)
These requirements do seem somewhat generic: even on a system which has a
different way to make a call could still create a local stub function, and
give you the address of that. However, “unnamed address” isn’t a good
name, because it doesn’t capture the first requirement, only the second.
I generally agree with Chris that it’d be nice to move towards being
able to represent target-specific relocations. That said, in this
specific case I completely agree with James: this is a portable
concept that shouldn’t be relegated to a bunch of target-specific
relocations. There’s no actual requirement that the function be the
one generated for PLT import; it just needs to be a function that’s
semantically equivalent to another.
Hi all. Are there any thoughts on the new name idea (“dso_local_stub” or
“dso_local_unnamed_stub”)? I’d like to see if I can move forward with my
patch.
There’s an interesting related idea here of emitting a reference to
a GOT entry. We do this in Swift by just emitting an unnamed_addr
private constant global variable that’s initialized to a particular
symbol, and the backend is typically clever enough to recognize the
pattern and use the relocations for a GOT entry. That is similarly
a portable concept: at worst, a backend can just make a normal global
variable. So at the very least, that’s something to consider in your
design.
We could try to apply that same approach to stubs. Pattern-matching
a short function body might be a lot to ask (and can’t always be done
if e.g. there are varargs or inalloca arguments), but we could just
set something on a Function that says it’s defined to be semantically
equivalent to some other Function. This would also support cases
like when two different symbols have to be exported but they’re known
to have the same effect. This can be done today with aliases, but
there are actually a lot of tricky object-file and linker restrictions
on aliases, and it would be nice if the frontend could just say
“this function has the same semantics as this other one” and let the
backend figure out whether an alias is possible or if it requires a
stub.
One virtue of making a new kind of Constant is that it’s naturally
a lot easier to test, though. For example, a v-table initializer
would just be a sequence of dso_local_stubs of functions. With
an alias-stub, the v-table initializer itself would just have a
sequence of fake functions, which you’d then have to separately test
to make sure they had the right equivalency link to the true target.
I’m wondering if it’s an issue that the name dso_local_stub is
specific to functions. The basic concept of “give me something
equivalent to this” also works for variables. Now, obviously there
are a ton of cases where it doesn’t work semantically for
variables:
- if the variable is mutable
- if the variable’s address is important
- if you don’t know the variable’s contents statically (unless the
loader supports something like a copy relocation, and you do at
least know the variable’s size)
But there are still cases where those things don’t hold: maybe
there’s some shared constant structure you’d like to use (like a
v-table), and you’d prefer to use the same copy as other people,
but only if it doesn’t require extra dynamic linking. If
you wanted to leave room for that kind of thing, you could use
the name dso_local_equivalent.
John.
I see. So we can extend this to accept global constant variables. On platforms that support it, we can lower to a GOT reference. I’m not sure what a generic case could be for other targets, but I guess we could explicitly leave it to the backend to decide by using the VK_GOTPCREL variant kind (MCSymbolRefExpr::create(Symbol, MCSymbolRefExpr::VK_GOTPCREL, Ctx)), which in the end may even use a GOT reference. I think this should still respect having a link-time constant offset and represent an insignificant address.
Using dso_local_equivalent sounds like a good idea. It looks like this could also be used by the RTTI component for the vtable. Right now the relative vtables ABI uses a proxy dso_local global for the RTTI component that just points to the original RTTI global, but accessing that global requires an extra indirection. For now I’d like to just implement support for functions, but can work on global constants afterwards.
I see. So we can extend this to accept global constant variables. On
platforms that support it, we can lower to a GOT reference. I'm not sure
what a generic case could be for other targets, but I guess we could
explicitly leave it to the backend to decide by using the `VK_GOTPCREL`
variant kind (`MCSymbolRefExpr::create(Symbol,
MCSymbolRefExpr::VK_GOTPCREL, Ctx)`), which in the end may even use a GOT
reference. I think this should still respect having a link-time constant
offset and represent an insignificant address.
Using `dso_local_equivalent` sounds like a good idea. It looks like this
could also be used by the RTTI component for the vtable. Right now the
relative vtables ABI uses a proxy dso_local global for the RTTI component
that just points to the original RTTI global, but accessing that global
requires an extra indirection. For now I'd like to just implement support
for functions, but can work on global constants afterwards.
Yes, I think that’s reasonable. The global-variable case is more
complicated and in its full generality would have to depend on unportable
features like copy relocations.
John.