Hi Nick,
The main use case I’ve seen is that it makes writing generic test cases for ‘opt’ easier in that it’s not necessary to specify a target triple on the command line or have a data layout in the .ll/.bc file. That is, in my experience, it’s more for convenience and perhaps historical layering considerations.
I have no philosophical objection to the direction you’re suggesting.
For modules without a data layout, use the host machine as you suggest. That’s consistent with what already happens with llc, so extending that to opt and other such tools seems reasonable to me.
On 30 Jan 2014, at 00:04, Nick Lewycky <nlew...@google.com> wrote:Unfortunately, reproducibility suffers. You commit a change, a test fails on two buildbots but passes on all of the others and on your local system. Now what do you do? I've already hit this problem in clang, with host-defined tool search paths leaking into the tests and causing them to fail on Windows only. It's hard to fix a bug that causes a buildbot failure if you can't reproduce it. At the very least, the target / data layout should be in the failure message that the test suite generates in case of failure so that you can reproduce it locally if a buildbot reports failure.
> This is also what many clang tests do, where TUs get parsed using the host triple. If we keep target datalayout out of the test files and fill it in with the host's information, then our test coverage expands as our buildbot diversity grows, which is a neat property.
On 30 Jan 2014, at 00:04, Nick Lewycky <nlew...@google.com> wrote:Unfortunately, reproducibility suffers. You commit a change, a test fails on two buildbots but passes on all of the others and on your local system. Now what do you do?
> This is also what many clang tests do, where TUs get parsed using the host triple. If we keep target datalayout out of the test files and fill it in with the host's information, then our test coverage expands as our buildbot diversity grows, which is a neat property.
I've already hit this problem in clang, with host-defined tool search paths leaking into the tests and causing them to fail on Windows only. It's hard to fix a bug that causes a buildbot failure if you can't reproduce it. At the very least, the target / data layout should be in the failure message that the test suite generates in case of failure so that you can reproduce it locally if a buildbot reports failure.
On 1/29/14 3:40 PM, Nick Lewycky wrote:Nick, I don't have a current system in place, but I do want to put forward an alternate perspective.
The LLVM Module has an optional target triple and target datalayout. Without them, an llvm::DataLayout can't be constructed with meaningful data. The benefit to making them optional is to permit optimization that would work across all possible DataLayouts, then allow us to commit to a particular one at a later point in time, thereby performing more optimization in advance.
This feature is not being used. Instead, every user of LLVM IR in a portability system defines one or more standardized datalayouts for their platform, and shims to place calls with the outside world. The primary reason for this is that independence from DataLayout is not sufficient to achieve portability because it doesn't also represent ABI lowering constraints. If you have a system that attempts to use LLVM IR in a portable fashion and does it without standardizing on a datalayout, please share your experience.
We've been looking at doing late insertion of safepoints for garbage collection. One of the properties that we end up needing to preserve through all the optimizations which precede our custom rewriting phase is that the optimizer has not chosen to "hide" pointers from us by using ptrtoint and integer math tricks. Currently, we're simply running a verification pass before our rewrite, but I'm very interested long term in constructing ways to ensure a "gc safe" set of optimization passes.
One of the ways I've been thinking about - but haven't actually implemented yet - is to deny the optimization passes information about pointer sizing.
Under the assumption that an opto pass can't insert an ptrtoint cast without knowing a safe integer size to use, this seems like it would outlaw a class of optimizations we'd be broken by.
My understanding is that the only current way to do this would be to not specify a DataLayout. (And hack a few places with built in assumptions. Let's ignore that for the moment.) With your proposed change, would there be a clean way to express something like this?
p.s. From reading the mailing list a while back, I suspect that the SPIR folks might have similar needs. (i.e. hiding pointer sizes, etc..) Pure speculation on my part though.
On 30 January 2014 02:10, David Chisnall <David.C...@cl.cam.ac.uk> wrote:
On 30 Jan 2014, at 00:04, Nick Lewycky <nlew...@google.com> wrote:Unfortunately, reproducibility suffers. You commit a change, a test fails on two buildbots but passes on all of the others and on your local system. Now what do you do?
> This is also what many clang tests do, where TUs get parsed using the host triple. If we keep target datalayout out of the test files and fill it in with the host's information, then our test coverage expands as our buildbot diversity grows, which is a neat property.
There's two issues here. One is what to do if we encounter a .ll/.bc with no target data. We're obliged to support llvm 3.0 bitcode files, so we need to have an answer to this question.Second is what to do in our test suite. If the answer to the first question is "make it use the host target data" then the second part is a choice either to leave the tests with no explicit layout and thereby use the host target, or to require that tests in the testsuite specify their datalayout. The tradeoff is that in one case we get more coverage across different machines, and in the other case we get better reproducibility, which is important for a regression suite or for a new user to verify that their build of llvm is valid.
On 30 January 2014 09:55, Philip Reames <list...@philipreames.com> wrote:
On 1/29/14 3:40 PM, Nick Lewycky wrote:Nick, I don't have a current system in place, but I do want to put forward an alternate perspective.
The LLVM Module has an optional target triple and target datalayout. Without them, an llvm::DataLayout can't be constructed with meaningful data. The benefit to making them optional is to permit optimization that would work across all possible DataLayouts, then allow us to commit to a particular one at a later point in time, thereby performing more optimization in advance.
This feature is not being used. Instead, every user of LLVM IR in a portability system defines one or more standardized datalayouts for their platform, and shims to place calls with the outside world. The primary reason for this is that independence from DataLayout is not sufficient to achieve portability because it doesn't also represent ABI lowering constraints. If you have a system that attempts to use LLVM IR in a portable fashion and does it without standardizing on a datalayout, please share your experience.
We've been looking at doing late insertion of safepoints for garbage collection. One of the properties that we end up needing to preserve through all the optimizations which precede our custom rewriting phase is that the optimizer has not chosen to "hide" pointers from us by using ptrtoint and integer math tricks. Currently, we're simply running a verification pass before our rewrite, but I'm very interested long term in constructing ways to ensure a "gc safe" set of optimization passes.
As a general rule passes need to support the whole of what the IR can support. Trying to operate on a subset of IR seems like a losing battle, unless you can show a mapping from one to the other (ie., using code duplication to remove all unnatural loops from IR, or collapsing a function to having a single exit node).
What language were you planning to do this for? Does the language permit the user to convert pointers to integers and vice versa? If so, what do you do if the user program writes a pointer out to a file, reads it back in later, and uses it?
One of the ways I've been thinking about - but haven't actually implemented yet - is to deny the optimization passes information about pointer sizing.
Right, pointer size (address space size) will become known to all parts of the compiler. It's not even going to be just the optimizations, ConstantExpr::get is going to grow smarter because of this, as lib/Analysis/ConstantFolding.cpp merges into lib/IR/ConstantFold.cpp. That is one of the major benefits that's driving this. (All parts of the compiler will also know endian-ness, which means we can constant fold loads, too.)
Under the assumption that an opto pass can't insert an ptrtoint cast without knowing a safe integer size to use, this seems like it would outlaw a class of optimizations we'd be broken by.
Optimization passes generally prefer converting ptrtoint and inttoptr to GEPs whenever possible.
I expect that we'll end up with *fewer* ptr<->int conversions with this change, because we'll know enough about the target to convert them into GEPs.
My understanding is that the only current way to do this would be to not specify a DataLayout. (And hack a few places with built in assumptions. Let's ignore that for the moment.) With your proposed change, would there be a clean way to express something like this?
I think your GC placement algorithm needs to handle inttoptr and ptrtoint, whichever way this discussion goes. Sorry. I'd be happy to hear others chime in -- I know I'm not an expert in this area or about GCs -- but I don't find this rationale compelling.
p.s. From reading the mailing list a while back, I suspect that the SPIR folks might have similar needs. (i.e. hiding pointer sizes, etc..) Pure speculation on my part though.
The SPIR spec specifies two target datalayouts, one for 32 bits and one for 64 bits.
Philip
Nick
Java - which does not permit arbitrary pointer manipulation. (Well, without resorting to mechanism like JNI and sun.misc.Unsafe. Doing so would be explicitly undefined behavior though.) We also use raw pointer manipulations in our implementation (which is eventually inlined), but this happens after the safepoint insertion rewrite.On 1/31/14 5:23 PM, Nick Lewycky wrote:
On 30 January 2014 09:55, Philip Reames <list...@philipreames.com> wrote:
On 1/29/14 3:40 PM, Nick Lewycky wrote:Nick, I don't have a current system in place, but I do want to put forward an alternate perspective.
The LLVM Module has an optional target triple and target datalayout. Without them, an llvm::DataLayout can't be constructed with meaningful data. The benefit to making them optional is to permit optimization that would work across all possible DataLayouts, then allow us to commit to a particular one at a later point in time, thereby performing more optimization in advance.
This feature is not being used. Instead, every user of LLVM IR in a portability system defines one or more standardized datalayouts for their platform, and shims to place calls with the outside world. The primary reason for this is that independence from DataLayout is not sufficient to achieve portability because it doesn't also represent ABI lowering constraints. If you have a system that attempts to use LLVM IR in a portable fashion and does it without standardizing on a datalayout, please share your experience.
We've been looking at doing late insertion of safepoints for garbage collection. One of the properties that we end up needing to preserve through all the optimizations which precede our custom rewriting phase is that the optimizer has not chosen to "hide" pointers from us by using ptrtoint and integer math tricks. Currently, we're simply running a verification pass before our rewrite, but I'm very interested long term in constructing ways to ensure a "gc safe" set of optimization passes.
As a general rule passes need to support the whole of what the IR can support. Trying to operate on a subset of IR seems like a losing battle, unless you can show a mapping from one to the other (ie., using code duplication to remove all unnatural loops from IR, or collapsing a function to having a single exit node).
What language were you planning to do this for? Does the language permit the user to convert pointers to integers and vice versa? If so, what do you do if the user program writes a pointer out to a file, reads it back in later, and uses it?
We strictly control the input IR. As a result, I can insure that the initial IR meets our subset requirements. In practice, all of the opto passes appear to preserve these invariants (i.e. not introducing inttoptr), but we'd like to justify that a bit more.I would argue that all of the pieces you mentioned are performing optimizations. :) However, the exact semantics are unimportant for the overall discussion.
One of the ways I've been thinking about - but haven't actually implemented yet - is to deny the optimization passes information about pointer sizing.
Right, pointer size (address space size) will become known to all parts of the compiler. It's not even going to be just the optimizations, ConstantExpr::get is going to grow smarter because of this, as lib/Analysis/ConstantFolding.cpp merges into lib/IR/ConstantFold.cpp. That is one of the major benefits that's driving this. (All parts of the compiler will also know endian-ness, which means we can constant fold loads, too.)This is good to hear and helps us.
Under the assumption that an opto pass can't insert an ptrtoint cast without knowing a safe integer size to use, this seems like it would outlaw a class of optimizations we'd be broken by.
Optimization passes generally prefer converting ptrtoint and inttoptr to GEPs whenever possible.Er, I'm confused by this. Why would not knowing the size of a pointer case a GEP to be converted to a ptr <-> int conversion?
I expect that we'll end up with *fewer* ptr<->int conversions with this change, because we'll know enough about the target to convert them into GEPs.
Or do you mean that after the change conversions in the original input IR are more likely to be recognized?The key assumption I didn't initially explain is that the initial IR couldn't contain conversions. With that added, do you still see concerns? I'm fairly sure I don't need to handle general ptr <-> int conversions. If I'm wrong, I'd really like to know it.
My understanding is that the only current way to do this would be to not specify a DataLayout. (And hack a few places with built in assumptions. Let's ignore that for the moment.) With your proposed change, would there be a clean way to express something like this?
I think your GC placement algorithm needs to handle inttoptr and ptrtoint, whichever way this discussion goes. Sorry. I'd be happy to hear others chime in -- I know I'm not an expert in this area or about GCs -- but I don't find this rationale compelling.
We're supposed to have the llvm.gcroots intrinsic for this purpose, but you note that it prevents gc roots from being in registers (they must be in memory somewhere, usually on the stack), and that fixing it is more work than is reasonable.
Your IR won't do any shifty pointer-int conversion shenanigans, and you want some assurance that an optimization won't introduce them, or that if one does then you can call it out as a bug and get it fixed. I think that's reasonable, but I also think it's something we need to put forth before llvm-dev.
Note that pointer-to-int conversions aren't necessarily just the ptrtoint/inttoptr instructions (and constant expressions), there's also casting between { i64 }* and { i8* }* and such. Are there legitimate reasons an optz'n would introduce a cast? I think that anywhere in the mid-optimizer, conflating integers and pointers is only going to be bad for both the integer optimizations and the pointer optimizations.
It may make sense as part of lowering -- suppose we find two alloca's, one i64 and one i8* and find that their lifetimes are distinct, and i64 and i8* are the same size, so we merge them. Because of how this would interfere, I don't think this belongs anywhere in the mid-optimizer, it would have to happen late, after lowering. That suggests that there's a point in the pass pipeline where the IR is "canonical enough" that this will actually work.
Is that reasonable? Can we actually guarantee that, that any pass which would break this goes after a common gc-root insertion spot? Do we need (want?) to push back and say "no, sorry, make GC roots better instead"?
Splitting out a conversation which started in "make DataLayout a mandatory part of Module" since the topic has decidedly changed. This also relates to the email "RFC: GEP as canonical form for pointer addressing" I just sent.
On 02/10/2014 05:25 PM, Nick Lewycky wrote:
...
We're supposed to have the llvm.gcroots intrinsic for this purpose, but you note that it prevents gc roots from being in registers (they must be in memory somewhere, usually on the stack), and that fixing it is more work than is reasonable.
This is slightly off, but probably close to what I actually said even if not quite what I meant. :)
I'm going to skip this and respond with a fuller explanation Monday. I'd written an explanation once, realized it was wrong, and decided I should probably revisit when fully awake.
Fundamentally, I believe that gc.roots could be made to work, even with decent (but not optimal) performance in the end. We may even contribute some patches towards fixing issues with the gc.root mechanism just to make a fair comparison. I just don't believe it's the right approach or the best way to reach the end goal.
On 02/24/2014 11:27 AM, Andrew Trick wrote:
Ah, okay. I think get where you're coming from.
On Feb 24, 2014, at 11:17 AM, Philip Reames <list...@philipreames.com> wrote:
On 02/24/2014 12:45 AM, Andrew Trick wrote:
Andy, I'm not clear what you're trying to say here. Could you rephrase? In particular, what do you mean by "call to invoke GC"?
On Feb 21, 2014, at 10:37 AM, Philip Reames <list...@philipreames.com> wrote:
On 02/14/2014 05:55 PM, Philip Reames wrote:
Splitting out a conversation which started in "make DataLayout a mandatory part of Module" since the topic has decidedly changed. This also relates to the email "RFC: GEP as canonical form for pointer addressing" I just sent.So, not quite on Monday, but I did get around to writing up an explanation of what's wrong with using gcroot. It turned out to be much longer than I expected, so I turned it into a blog post:
On 02/10/2014 05:25 PM, Nick Lewycky wrote:
This is slightly off, but probably close to what I actually said even if not quite what I meant. :)...
We're supposed to have the llvm.gcroots intrinsic for this purpose, but you note that it prevents gc roots from being in registers (they must be in memory somewhere, usually on the stack), and that fixing it is more work than is reasonable.
I'm going to skip this and respond with a fuller explanation Monday. I'd written an explanation once, realized it was wrong, and decided I should probably revisit when fully awake.
Fundamentally, I believe that gc.roots could be made to work, even with decent (but not optimal) performance in the end. We may even contribute some patches towards fixing issues with the gc.root mechanism just to make a fair comparison. I just don't believe it's the right approach or the best way to reach the end goal.
http://www.philipreames.com/Blog/2014/02/21/why-not-use-gcroot/
The very short version: gcroot loses roots (for any GC) due to bad interaction with the optimizer, and gcroot doesn't capture all copies of a pointer root which fundamentally breaks collectors which relocate roots. The only way I know to make gcroot (in its current form) work reliably for all collectors is to insert safepoints very early, which has highly negative performance impacts. There are some (potentially) cheaper but ugly hacks available if you don't need to relocate roots.
There's also going to be a follow up post on implementation problems, but that's completely separate from the fundamental problems.
Thanks for the writeup. FWIW my understanding of gcroot has always been that the call to invoke GC is “extern” and not readonly, so we can’t do store->load forwarding on the escaped pointer across it. I have never used gcroot myself.
I mean a call site that we think of as a safepoint could potentially call to the runtime and block while GC runs. We can’t let LLVM optimize hoist loads or sink stores across that call.
For call safepoints, if you assume the call itself prevents the optimization, you're mostly okay. This is problematic if you want to have a safepoint on a read-only call (for example), but could be hacked around.
The problem comes up with backedge, function entry, and function return safepoints. Given there is no example in tree (or out of tree that I know of) which uses these, it's a little hard to tell how it's supposed to work. My belief is that the findCustomSafePoints callback on GCStrategy is supposed to insert these. The problem is that this pass is a MachineFunction pass and this runs long after optimization.