[llvm] r209007 - Fix most of PR10367.

Nick Lewycky nicholas at mxc.ca
Sat May 17 21:03:27 PDT 2014


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.

>>> 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.

This also means that we won't have as strong a "portable IR" story as we 
do today, but that's just accepting reality. ABI concerns are baked into 
llvm IR, every user of LLVM as a portable IR specifies target data and 
data layout.

We'll have a common IR for &symbol and probably have one for &symbol + 
byte offset, but even that's not certain since what type should our byte 
offset have? If we set i32, and say that this is always available, then 
we've just ensure that nobody can use llvm on their 16-bit embedded 
platform. On the other hand, maybe that's okay? I don't have a full 
proposal or else I would've taken it to llvmdev already, but this is 
exactly the direction we ought to go.

Once we have this in place, we migrate away from, then remove ConstantExpr.

Nick



More information about the llvm-commits mailing list