[llvm] r209007 - Fix most of PR10367.

[John McCall]

On May 17, 2014, at 9:03 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
> John McCall wrote:
>> On May 17, 2014, at 7:54 PM, Rafael Espíndola<rafael.espindola at gmail.com>  wrote:
>>>> That argument works when it’s really just “allowing two different types”, but it breaks down the more features you add to aliases.  If aliases support bitcasts (by just allowing the type to be different) and geps (by allowing an offset to be given) and address-space conversions (by allowing an explicit address space that doesn’t match the base)
>>> 
>>> That is part of the type.
>>> 
>>>> then it sure sounds to me like you’re just repeating a substantial subset of expressive capability of ConstantExpr, and there isn’t a huge benefit to this approach over just semantically restricting what can appear in an alias.
>>> 
>>> It is not a substantial part. There is a *lot* more in ConstantExpr.
>>> As an extreme example, ConstantExpr can trap.
>> 
>> 1. That is clearly a mistake.
>> 
>> 2. You are not going to be able to fix that mistake with a change in
>> representation.  In general, the operands of a trapping operation are
>> otherwise legal constants, so the only way to intrinsically remove the
>> potential for trapping constants is to remove all operations from
>> ConstantExpr that could possibly trap on any operand.  I don’t think
>> you have community support for that, so assuming that that’s the
>> inevitable direction is not actually reasonable.
>> 
>> 3. There is a much more obvious solution: we should just document
>> that ConstantExprs which are guaranteed to trap are illegal, and then
>> check that in the verifier.  Constant propagation should already be
>> doing this analysis to decide that it can safely turn an instruction
>> into a constant.
> 
> Concisely, this saves almost no work over removing ConstantExpr entirely but falls quite a bit short of being a comprehensive fix. You have all the difficulty of dealing with autoupgrading code that uses ConstantExpr udiv &global1, &global2, but without going all the way to having a sane new design.

1. I don’t think we need to be able to autoupgrade code that we declare is ill-formed.
2. Your design is not sane.

>>>> It also seems to me like the set of expressions supported by global aliases is only likely to increase.  It’s a pretty natural way to express absolute symbols, for example.
>>>> 
>>>> Note that there are already practical restrictions on the kinds of ConstantExpr that can appear in various places — e.g. you can make a ConstantExpr that divides the address of a global variable by 7, but there aren’t any relocations that support that, so…
>>> 
>>> So it is better to have a more restrictive representation.
>> 
>> I don’t see why.  Your more restrictive representation is just going to
>> arbitrarily limit the ability of frontends and backends to achieve things
>> that your representation doesn’t realize are possible.  LLVM IR should
>> not be in the business of knowing what common object file formats
>> are capable of expressing.
> 
> On the contrary, that is precisely the plan (my plan). Why shouldn't it?
> 
> Suppose we come up with a new set of constants that represent the set of relocations available in the targets we support. This would permit us to generate optimal code for those targets, instead of our current situation of producing worse code on Linux/ELF because Mach-O doesn't support certain kinds of relocation.

Your proposal is seriously to introduce relocations as a first-class concept in LLVM IR?  What possible benefit does that have over just allowing them to be expressed naturally as ConstantExprs and then ensuring that front-ends and transforms are smart about not introducing ConstantExprs that the tool chain / target can’t handle?  You know, like they already do today, both for relocations and for a thousand other features that might be impossible on the target.

It’s not as if using magic named relocations is going to actually guarantee anything, since different classes of “relocations” are legal in different situations and given different sets of operands, and since tool chains do evolve and improve, as do dynamic linkers.

The problem you’ve identified is because LLVM IR is being unnecessarily paternalistic.  The solution is not to introduce extra obstacles and be even more paternalistic.

John.