[llvm] r209007 - Fix most of PR10367.

Nick Lewycky nicholas at mxc.ca
Sun May 18 00:50:34 PDT 2014

John McCall wrote:
> On May 17, 2014, at 10:42 PM, Nick Lewycky<nicholas at mxc.ca>  wrote:
>> John McCall wrote:
>>> On May 17, 2014, at 9:03 PM, Nick Lewycky<nicholas at mxc.ca>   wrote:
>>>> 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?
>> Yes, filling the role ConstantExpr fills now.
>>> 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.
>> I need to pull this apart a bit to answer it. Why get rid of ConstantExpr?
>> ConstantExpr has the wrong design. The constants available in LLVM are typed bags of bits, the address of a function, the address of a global variable, an alias, and relocations. ConstantExpr is modeled as a mirror of the set of LLVM instructions -- and which of those things in the previous list correspond to instructions? Immediately we see problems with the model, there are instructions aren't mirrored -- nobody has a call ConstantExpr, or a load, or unreachable. Trapping is just another problem on the list.
> …because control flow and side effects aren't constants.  Are there side-effectful relocations you’re concerned about representing?  What is the use of a side effect which occurs in an unreliable order at load time?

Yeah, there's IFUNC. Good uses include checking the cpuid or cache line 
size and resolving the relocation to the more efficient function 
implementation or lookup table. Bad uses include side-effecting 
functions, my personal favourite being a scheme that requires the 
dynamic loader to be re-entrant (sigh).

>> ConstantExpr's also try to offer other guarantees, like guarantees they've been canonicalized. We can do this because unlike instructions there's a centralized getter. The complexity of constantexpr canonicalization gets worse as the matching instructions change. When should 'add x, y' and 'add nsw x, y' be canonicalized to the same constantexpr, when 'x' and 'y' aren't concrete yet? There are places in LLVM which rely on the former fact that certain subsets of CEs will get canonicalized, and which now miscompile because instructions got more complex. For a personal pet peeve, GlobalOpt lost its war when getelementptr grew the ability to have arbitrary precision integers for its indices -- "gep x, i5 0" and "gep x, i6 0" as two different constant expressions. (And how would we canonicalize those? When we don't have DataLayout and thus don't know the pointer width? To i1 0 because that's the smallest size that '0' fits in?) GlobalOpt used to work, an implementation of symb!
 olic execu
tion that relies on ConstantExpr (checked for simplicity) as keys to simulated memory. We don't see the miscompiles with clang because these go away thanks to a second constant folder that requires data layout to be present.
> Then LLVM pervasively mis-uses canonicalization.

This is a great point of discussion. You can design a compiler IR either 
way, deciding that we'll go with the notion that this is okay behaviour 
or the notion that llvm is doing this wrong, and either way is 
internally consistent. In short, should globalopt be able to do what I 
say it does, modeling memory with IR constants? I'm taking the path that 
the answer is yes, considering what I would need to build if the answer 
were 'no' and why that's better or worse.

   I don’t see how any of these problems are solved by target-specific 
relocations, which (1) can overlap in ways that analyses must be aware 
of and yet (2) inherently can’t be canonicalized or even interpreted 
without, not just data layout, but a complete target available.
>> Now, how do we make sure we don't build invalid ConstantExpr's? Well currently GlobalOpt just refuses to build anything too complicated for the intersection of our supported platforms. As I alluded to in my last email, there are cases globalopt drops on the floor that GCC (on Linux) optimizes just fine, because globalopt doesn't know that ELF can do things Mach-O can't. If we want to get these right, then we need to make our constant expr creation depend on the target. So that's step one: make target information a mandatory part of the Module, instead of optional.
> If “target information” means “know the exact semantics of an arbitrary list of target- and toolchain- specific relocations”, not just “what’s the target data layout”, that’s a very significant new requirement.

Truth. That is a downside.

> ConstantExprs can be meaningfully interpreted by target-independent code, even if they can’t always be validly generated by it.  Or really, they can always be generated by target-independent code (assuming it can be proven not to trap), they just can’t necessarily be used in certain places.

Oh that reminded me. Getting rid of ConstantExpr's lets us get rid of 
much of the llvm::Operator layer and the undefined behaviour it brings, 
caused by people trying to treat instructions and constantexpr's with 
the same code.

>>> 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.
>> What value is there in supporting arbitrarily complex or long constant expressions?
>> Either way, we need to teach the compiler about a new relocation before we can use it. You aren't gaining anything by deciding to lower CEs to relocations in the backend. Even if your plan is to keep CEs in the IR then feed that IR to a newer llvm whose backend knows about the new relocations, the only difference is that the newer llvm would have to try refolding relocations when loading older IR.
> 1. It’s an optimization.  There’s no “have to” here.
> 2. That code looks exactly the same whether you’re using ConstantExpr or ConstantRelocation.
> Both of our proposals involve target-specific IR emission/transformation using target-specific information to produce a constant that’s only valid on specific targets.  The only difference is that you want to make constant completely opaque to target-independent code, even when it’s something obvious like “take the low 17 bits of this pointer”, because you find the possibility that somebody might accidentally form such a constant — and thus require the backend to reject the code — aesthetically unappealing.  In contrast, I feel that allowing the IR to express things that can’t necessarily be emitted by the backend in every context preserves a lot of very useful flexibility.

Then our proposals are indeed similar. I see no need for these objects 
to be opaque, I expect optimizations to learn handle them like they 
handle any other instruction or Constant. We may simplify that by the 
addition of common base classes that permit unified handling of common 

> If I want to use a new relocation, in either proposal, I have to change my frontend/transform to emit a constant that follows a certain pattern, and I have to change the backend to recognize that pattern and use the relocation.  Your proposal also requires me to change some sort of intrinsics-like table in IR and make sure that a bunch of useful target-independent transforms know how to exploit that table to do basic analyses involving my constant.
>>> 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.
>> The problem I've identified is that LLVM IR is too high level and is providing an abstraction that doesn't match what actually happens at the low level. Replacing the IR representation corrects the problem by making the IR match reality.
> We strongly disagree on this design, and that’s fine.  But I do not see why it matters, because you not only haven’t proposed it, but you haven’t even fleshed it out to yourself yet, which is, again, fine as far as your proposal for ConstantExpr goes.  The problem is that you and Rafael have decided to go ahead and use the basic thrust of this less-than-half-baked proposal to completely redesign an existing LLVM feature in a way that intentionally constrains and complicates future extensions.  That’s using trunk as an experimental lab for IR design, and it seems totally inappropriate to me.

Right. Dropping the rest, I fundamentally agree here. It's nice when a 
feature dovetails with other plans, but a change being made now has to 
make sense with llvm as it stands now (consider the discussion to make 
datalayout mandatory -- it makes sense for llvm regardless of whether we 
go down the path of target-specific IR nodes).

That said, the same applies to your proposed solution of making the 
verifier fail any constant expression with C->canTrap() true. By the 
way, constant propagation does not do this analysis when deciding 
whether it can turn an instruction into a constant, it just always turns 
instructions with constant arguments into constants. This means that we 
can't safely sink/hoist constantexpr's around, and indeed, we've taught 
the optimizers to check whether operands have trapping constant 
expressions before moving instructions around. Mostly. I bet you could 
find bugs.


More information about the llvm-commits mailing list