llvm dev - Aug 2020 - [RFC][LLVM] New Constant type for representing function PLT entries

> -----Original Message-----
> From: Fāng-ruì Sòng <maskray at google.com>
> Sent: Friday, August 21, 2020 4:04 PM
> To: Eli Friedman <efriedma at quicinc.com>
> Cc: Leonard Chan <leonardchan at google.com>; llvm-dev at
lists.llvm.org
> Subject: [EXT] Re: [llvm-dev] [RFC][LLVM] New Constant type for
> representing function PLT entries
>
> On Fri, Aug 21, 2020 at 3:32 PM Eli Friedman <efriedma at
quicinc.com> wrote:
> >
> > > -----Original Message-----
> > > From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On
Behalf Of Fangrui
> > > Song via llvm-dev
> > > Sent: Thursday, August 20, 2020 10:18 PM
> > > To: Leonard Chan <leonardchan at google.com>
> > > Cc: llvm-dev <llvm-dev at lists.llvm.org>
> > > Subject: [EXT] Re: [llvm-dev] [RFC][LLVM] New Constant type for
> > > representing function PLT entries
> > >
> > Maybe it would make sense to introduce a GlobalValue to represent
this,
> > along the lines of GlobalIFunc?  I guess the end result isn't a
lot different
> > from the original proposal: you still end up with a Constant that
represents
> > the PLT entry.  But I think it would fit more smoothly into existing
> > optimizations that understand GlobalValues.  And it would make it
clear that
> > importing one from another IR module might be a non-trivial operation.
(I
> > don't think we actually do cross-DSO optimizations at the moment,
but it's
> > not outside the realm of possibility.)
> >
> A new subclass of GlobalIndirectSymbol? Looks fine. Do you think the
> name "plt" should be moved from the name of the subclass to an
> argument of the syntax (like alias/ifunc)?
The IR concept could be generalized so it doesn't actually depend on the
object format having a PLT.  For example, it could be used to refer to an import
thunk on Windows.  So I'm not sure we want to name the IR concept after a
"PLT".

LLVM IR currently has the dso_local concept, so maybe we want to leverage that
in the name. Maybe something like GlobalDSOLocalFunc?  (Not sure I'm really
happy with that exact name, but hopefully the idea makes sense.)

-Eli

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 at . 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 at plt), a
thunk, or anything that’s equivalent to the resolved function at runtime.

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.

Is there anything else that should be addressed? Hopefully this addresses
some concerns.

On Fri, Aug 21, 2020 at 6:14 PM Eli Friedman <efriedma at quicinc.com>
wrote:
> > -----Original Message-----
> > From: Fāng-ruì Sòng <maskray at google.com>
> > Sent: Friday, August 21, 2020 4:04 PM
> > To: Eli Friedman <efriedma at quicinc.com>
> > Cc: Leonard Chan <leonardchan at google.com>; llvm-dev at
lists.llvm.org
> > Subject: [EXT] Re: [llvm-dev] [RFC][LLVM] New Constant type for
> > representing function PLT entries
> >
> > On Fri, Aug 21, 2020 at 3:32 PM Eli Friedman <efriedma at
quicinc.com>
> wrote:
> > >
> > > > -----Original Message-----
> > > > From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On
Behalf Of
> Fangrui
> > > > Song via llvm-dev
> > > > Sent: Thursday, August 20, 2020 10:18 PM
> > > > To: Leonard Chan <leonardchan at google.com>
> > > > Cc: llvm-dev <llvm-dev at lists.llvm.org>
> > > > Subject: [EXT] Re: [llvm-dev] [RFC][LLVM] New Constant type
for
> > > > representing function PLT entries
> > > >
> > > Maybe it would make sense to introduce a GlobalValue to represent
this,
> > > along the lines of GlobalIFunc?  I guess the end result isn't
a lot
> different
> > > from the original proposal: you still end up with a Constant that
> represents
> > > the PLT entry.  But I think it would fit more smoothly into
existing
> > > optimizations that understand GlobalValues.  And it would make it
> clear that
> > > importing one from another IR module might be a non-trivial
> operation.  (I
> > > don't think we actually do cross-DSO optimizations at the
moment, but
> it's
> > > not outside the realm of possibility.)
> > >
> > A new subclass of GlobalIndirectSymbol? Looks fine. Do you think the
> > name "plt" should be moved from the name of the subclass to
an
> > argument of the syntax (like alias/ifunc)?
>
> The IR concept could be generalized so it doesn't actually depend on
the
> object format having a PLT.  For example, it could be used to refer to an
> import thunk on Windows.  So I'm not sure we want to name the IR
concept
> after a "PLT".
>
> LLVM IR currently has the dso_local concept, so maybe we want to leverage
> that in the name. Maybe something like GlobalDSOLocalFunc?  (Not sure
I'm
> really happy with that exact name, but hopefully the idea makes sense.)
>
> -Eli
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200824/0e14f698/attachment-0001.html>

Does anyone have more comments? Otherwise, I'll go ahead and update my
patch to reflect my update.

On Mon, Aug 24, 2020 at 6:47 PM Leonard Chan <leonardchan at google.com>
wrote:
> 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 at . 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 at plt), a
> thunk, or anything that’s equivalent to the resolved function at runtime.
>
> 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.
>
> Is there anything else that should be addressed? Hopefully this addresses
> some concerns.
>
> On Fri, Aug 21, 2020 at 6:14 PM Eli Friedman <efriedma at
quicinc.com> wrote:
>
>> > -----Original Message-----
>> > From: Fāng-ruì Sòng <maskray at google.com>
>> > Sent: Friday, August 21, 2020 4:04 PM
>> > To: Eli Friedman <efriedma at quicinc.com>
>> > Cc: Leonard Chan <leonardchan at google.com>; llvm-dev at
lists.llvm.org
>> > Subject: [EXT] Re: [llvm-dev] [RFC][LLVM] New Constant type for
>> > representing function PLT entries
>> >
>> > On Fri, Aug 21, 2020 at 3:32 PM Eli Friedman <efriedma at
quicinc.com>
>> wrote:
>> > >
>> > > > -----Original Message-----
>> > > > From: llvm-dev <llvm-dev-bounces at
lists.llvm.org> On Behalf Of
>> Fangrui
>> > > > Song via llvm-dev
>> > > > Sent: Thursday, August 20, 2020 10:18 PM
>> > > > To: Leonard Chan <leonardchan at google.com>
>> > > > Cc: llvm-dev <llvm-dev at lists.llvm.org>
>> > > > Subject: [EXT] Re: [llvm-dev] [RFC][LLVM] New Constant
type for
>> > > > representing function PLT entries
>> > > >
>> > > Maybe it would make sense to introduce a GlobalValue to
represent
>> this,
>> > > along the lines of GlobalIFunc?  I guess the end result
isn't a lot
>> different
>> > > from the original proposal: you still end up with a Constant
that
>> represents
>> > > the PLT entry.  But I think it would fit more smoothly into
existing
>> > > optimizations that understand GlobalValues.  And it would
make it
>> clear that
>> > > importing one from another IR module might be a non-trivial
>> operation.  (I
>> > > don't think we actually do cross-DSO optimizations at the
moment, but
>> it's
>> > > not outside the realm of possibility.)
>> > >
>> > A new subclass of GlobalIndirectSymbol? Looks fine. Do you think
the
>> > name "plt" should be moved from the name of the subclass
to an
>> > argument of the syntax (like alias/ifunc)?
>>
>> The IR concept could be generalized so it doesn't actually depend
on the
>> object format having a PLT.  For example, it could be used to refer to
an
>> import thunk on Windows.  So I'm not sure we want to name the IR
concept
>> after a "PLT".
>>
>> LLVM IR currently has the dso_local concept, so maybe we want to
leverage
>> that in the name. Maybe something like GlobalDSOLocalFunc?  (Not sure
I'm
>> really happy with that exact name, but hopefully the idea makes sense.)
>>
>> -Eli
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200827/1bf2fe1e/attachment.html>

On Aug 24, 2020, at 6:47 PM, Leonard Chan via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 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 at . 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 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200829/12bc0f2e/attachment.html>