On 29 January 2014 15:53, Jim Grosbach <grosbach at apple.com> wrote:> 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. >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. Nick On Jan 29, 2014, at 3:40 PM, Nick Lewycky <nlewycky at google.com> wrote:> > > 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. > > > > The cost to keeping this feature around is that we have to pass around > the DataLayout object in many places, test for its presence, in some cases > write different optimizations depending on whether we have DataLayout, and > in the worst case I can think of, we have two different canonical forms for > constant expressions depending on whether DL is present. Our canonical IR > is different with and without datalayout, and we have two canonicalizers > fighting it out (IR/ConstantFold.cpp and Analysis/ConstantFolding.cpp). > > > > I'm trying to force the issue. Either this is a useful feature to > maintain in which case I want to see a design on how to defer ABI decisions > until a later point in time, or else we do not support it and target triple > and target datalayout become a mandatory part of a valid Module again. I > think the correct direction is to make them mandatory, but this is a large > change that warrants debate. > > > > If we decide that target information should be a mandatory part of a > module, there's another question about what we should do with existing .bc > and .ll files that don't have one. Load in a default of the host machine? > > > > Nick > > > > _______________________________________________ > > LLVM Developers mailing list > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140129/f3c17a7a/attachment.html>
On 30 Jan 2014, at 00:04, Nick Lewycky <nlewycky at google.com> wrote:> 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.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. David
On Jan 30, 2014, at 2:10 AM, David Chisnall <David.Chisnall at cl.cam.ac.uk> wrote:> On 30 Jan 2014, at 00:04, Nick Lewycky <nlewycky at google.com> wrote: > >> 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. > > 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. > > David > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdevWhy not default to using a generic datalayout that just uses the defaults for everything?. There are already defaults, since not every option needs to be specified in it, you just don’t get them when you don’t have one at all. Some places without one already make some assumptions like that.
On Thu, Jan 30, 2014 at 2:10 AM, David Chisnall <David.Chisnall at cl.cam.ac.uk> wrote:> On 30 Jan 2014, at 00:04, Nick Lewycky <nlewycky at google.com> wrote: > > > 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. > > 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. >I don't think this will be a problem for opt or other LLVM tools. If opt has a dependence on the host's default triple and datalayout, reproducing the failure should be a simple matter of running the test locally with a manually specified triple. It doesn't have implicit header search paths or other weird host dependencies. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140130/48fa720c/attachment.html>
On 30 January 2014 02:10, David Chisnall <David.Chisnall at cl.cam.ac.uk>wrote:> On 30 Jan 2014, at 00:04, Nick Lewycky <nlewycky at google.com> wrote: > > > 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. > > 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?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. 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. >Exactly. As long as it's easy to grab the target datalayout from a buildbot, we can slap it into our .ll file and reproduce the failure. I can see both sides, and I know my preference, but I'd like to form consensus. Nick -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140131/bdbda942/attachment.html>