llvm dev - Oct 2016 - RFC: Absolute or "fixed address" symbols as immediate operands

> You get the code you want with
>
> @foo = external hidden global i8
>
> or
>
> @foo = external protected global i8
Sorry, not quiet. What you get is

leaq    foo(%rip), %rax

Since it is still assuming foo is a position in the file.

So yes, we need a way to declare an absolute value. If we can declare
it, we may as well use the same construct to define it.

I guess the only issue then is that the current implementation of
alias becomes even more needlessly generic :-(

Cheers,
Rafael

On Tue, Oct 25, 2016 at 2:55 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:
> > You get the code you want with
> >
> > @foo = external hidden global i8
> >
> > or
> >
> > @foo = external protected global i8
>
> Sorry, not quiet. What you get is
>
> leaq    foo(%rip), %rax
>
> Since it is still assuming foo is a position in the file.
>
> So yes, we need a way to declare an absolute value. If we can declare
> it, we may as well use the same construct to define it.
>
> I guess the only issue then is that the current implementation of
> alias becomes even more needlessly generic :-(
>
> Cheers,
> Rafael
>
So, to summarise the overall consensus on this thread:
- We should introduce a GlobalConstant class which supports definitions and
declarations
- It should derive from GlobalValue
- No type changes for GlobalValue (i.e. still required to be pointer type)
- To communicate the range we should use !range metadata and support it on
GlobalConstant as well as GlobalVariable

Although I am not convinced that this is the right representation I appear
to be in the minority here, so if this seems reasonable to folks I will go
ahead and implement it.

Thanks,
-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161025/5abc33d4/attachment.html>

On Tue, Oct 25, 2016 at 6:52 PM Peter Collingbourne <peter at pcc.me.uk>
wrote:
> On Tue, Oct 25, 2016 at 2:55 PM, Rafael Espíndola <
> rafael.espindola at gmail.com> wrote:
>
> > You get the code you want with
> >
> > @foo = external hidden global i8
> >
> > or
> >
> > @foo = external protected global i8
>
> Sorry, not quiet. What you get is
>
> leaq    foo(%rip), %rax
>
> Since it is still assuming foo is a position in the file.
>
> So yes, we need a way to declare an absolute value. If we can declare
> it, we may as well use the same construct to define it.
>
> I guess the only issue then is that the current implementation of
> alias becomes even more needlessly generic :-(
>
> Cheers,
> Rafael
>
>
> So, to summarise the overall consensus on this thread:
> - We should introduce a GlobalConstant class which supports definitions
> and declarations
> - It should derive from GlobalValue
> - No type changes for GlobalValue (i.e. still required to be pointer type)
> - To communicate the range we should use !range metadata and support it on
> GlobalConstant as well as GlobalVariable
>
> Although I am not convinced that this is the right representation I appear
> to be in the minority here, so if this seems reasonable to folks I will go
> ahead and implement it.
>
FWIW, I really dislike this plan.

We are essentially saying we have a GlobalConstant class which is a
pointer, but doesn't point to anything. Instead, it is a pointer that will
have a fixed "constant" address that can be manipulated and/or
exploited to
have a very particular integer value via ptrtoint.

I feel like we are going to tremendous lengths to avoid having a
non-pointer global value despite ... having a non-pointer global value. I
would like to understand why it is so problematic to just model this
directly and explicitly in the IR...

>
> Thanks,
> --
> --
> Peter
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161026/3aa9dbb6/attachment.html>

Responding to both of your emails in one, sorry for the delay:
> On Oct 25, 2016, at 11:20 AM, Peter Collingbourne <peter at
pcc.me.uk> wrote:
> I think there are a couple of additional considerations we should make
here:
> What are we trying to model? To me it's clear that GlobalConstant is
for modelling integers, not pointers. That alone may not necessarily be enough
to motivate a representational change, but…
I understand where you’re coming from, but I think we’re modeling three
different things, and disagreeing about how to clump them together.  The three
things I see in flight are:

1) typical globals that are laid out in some unknown way in the address space.
2) globals that may be tied to a specific knowable address range due to a
limited compilation model (e.g. a deeply embedded core) that fits into an
immedaite range (e.g. 0…255, 0…65536, etc).
3) Immediates that are treated as symbolic for CFI’s perspective (so they can’t
just be used as a literal immediate) that are resolved at link time, but are
known to have limited range.

There is also "4) immediates with an obvious known value”, but those are
obviously ConstantInt’s and not interesting to discuss here.

The design I’m arguing for is to clump #2 and #3 into the same group.  This can
be done one of two different ways, but both ways use the same “declaration side”
reference, which has a !range metadata attached to it.  The three approaches I
see are:

a) Introduce a new GlobalConstant definition, whose value is the concrete
address that the linker should resolve.
b) Use an alias as the definition, whose body is a ptrtoint constant of the same
value.
c) Use a zero size globalvariable with a range metadata specifying the exact
address decided.

I’m not very knowledgable about why approach b won’t work, but if it could, it
seems preferable because it fits in with our current model.  After that,
approach C seems like a nice fallback because it again doesn’t require major
extensions to our model.  Finally, approach A seems like the most explicit
model, but has the disadvantage of introducing a new IR concept that will be
rarely used (and thus prone to bugs due to it not being widely tested).
> If we make our representation fit the model, does that improve the quality
of the code? There's clearly been a long standing assumption that
GlobalValues are of pointer type. Breaking that assumption has in fact found
potential bugs (see e.g. my changes to ValueTracking.cpp in D25930, as well as
D25917 which was a requirement for the representational change), and seems like
it may help prevent bugs in the future, so I'd say that the answer is most
likely yes.
I’d argue the other side of it.  The quality of the code is higher if we have
invariants (like all globals are pointers) because that simplifies assumptions
by eliminating cases where “is a pointer” appears to be true, but isn’t actually
true in all cases.  I’m not an expert on CFI or how widely it will ultimately
impact the compiler hacker consciousness, but I’m pretty sure that the current
model for globals and functions will remain more prominent.  If you choose to
break this invariant, you’ll be continually swimming upstream against
assumptions made throughout the compiler, both in code written today but also in
code written in the future.

On Oct 25, 2016, at 6:52 PM, Peter Collingbourne <peter at pcc.me.uk>
wrote:
> 
> So, to summarise the overall consensus on this thread:
> - We should introduce a GlobalConstant class which supports definitions and
declarations
Why would globalconstant need to support declarations?  Seems like an external
global should be able to handle this case.
> - It should derive from GlobalValue
> - No type changes for GlobalValue (i.e. still required to be pointer type)
> - To communicate the range we should use !range metadata and support it on
GlobalConstant as well as GlobalVariable
> 
> Although I am not convinced that this is the right representation I appear
to be in the minority here, so if this seems reasonable to folks I will go ahead
and implement it.
I’m not trying to steamroll you over: what is your specific concern?

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161025/1d590327/attachment.html>

On Tue, Oct 25, 2016 at 6:52 PM, Peter Collingbourne <peter at pcc.me.uk>
wrote:
> So, to summarise the overall consensus on this thread:
> - We should introduce a GlobalConstant class which supports definitions
> and declarations
> - It should derive from GlobalValue
> - No type changes for GlobalValue (i.e. still required to be pointer type)
> - To communicate the range we should use !range metadata and support it on
> GlobalConstant as well as GlobalVariable
>
> Although I am not convinced that this is the right representation I appear
> to be in the minority here, so if this seems reasonable to folks I will go
> ahead and implement it.
>
I could go either way on GlobalConstant being a pointer or not. Keeping the
"globals are points" invariant is nice, but making LLVM IR more
accurately
model object file formats is also good.

If I had to choose, I think I would make GlobalConstant a pointer. I don't
think there are many interesting mid-level optimizations that this prevents.

One change I would make is to push the !range metadata into the
GlobalConstant object. The range information is what really unlocks the
interesting ISel optimizations.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161026/6e2f352c/attachment.html>