On Oct 22, 2009, at 7:25 AM, Kenneth Uildriks wrote:> On Wed, Oct 21, 2009 at 4:47 PM, Kenneth Uildriks <kennethuil at gmail.com> wrote: >>> I think it's more intuitive to have command-line information override >>> Module information. That's how llc works, for example. >>> >>> Also, is the argument to -defaulttarget a triple, an architecture name, >>> or a targetdata string? If it's a triple, it'd be nice to be consistent >>> with llc and call it -mtriple=. For an architecture name, -march=. >>> If it's a targetdata string, perhaps -targetdata= would be a good name. >>> >>> (As an aside, I wouldn't object to having llc's options renamed to >>> remove the leading 'm', as that seems to have been intended to follow >>> GCC's targeting options, and they aren't the same.) >>> >>> Dan >>> >>> >> >> The argument to -default-data-layout is a targetdata string. >> -no-default-data-layout means that no TargetData pass is added unless >> the module supplies a target data string. >> >> llvm-gcc always inserts targetdata. I'm wondering if the code it >> generates somehow depends on the assumption that 'opt' is taking its >> target data into account. As in, some of it uses absolute offsets and >> some of it uses pointer-indexing that gets affected by the targetdata. >> Anyway, it seemed safer to take the module's targetdata if it was >> built with targetdata included > > Note to self: wait at least 24 hours after soliciting feedback before > sending a patch. > > Anyway, after thinking about it, it should always be safe to override > the Module to remove the target data pass, even if it isn't safe to > override the Module to substitute different target data. But I still > think you should at least have the option to supply target data > *without* overriding whatever comes in from the module.In what situations would this be useful? Would it make sense to have opt issue an error if the module and the command-line have incompatible non-empty strings?> An even better question is: does it *ever* make sense to supply a > blanket default target data striing? If no target-data option is > supplied, wouldn't it be better to default to the target data for the > running host? Or would that break existing code and/or tests?It would break existing tests, which currently rely on an empty targetdata string being interpreted as historical sparc settings. But tests can be updated; it's more important to figure out how to make opt useful first. Dan
On Thu, Oct 22, 2009 at 3:54 PM, Dan Gohman <gohman at apple.com> wrote:>> Anyway, after thinking about it, it should always be safe to override >> the Module to remove the target data pass, even if it isn't safe to >> override the Module to substitute different target data. But I still >> think you should at least have the option to supply target data >> *without* overriding whatever comes in from the module. > > In what situations would this be useful?It is definitely useful to me to tell opt "don't use a target data string unless the module has one"... then I can feed it modules built without a target platform, or modules built with the target platform I'm using, and it'll do the best thing in both cases and break neither. "Use this string unless the module has a different one"... the module would only have a different one if it was compiled by llvm-gcc or some other front-end specifically targeting some other platform... and modules specifically targeting different platforms probably won't get thrown together in the same build, folder, or process. So that may not be as useful as I thought. In short, I definitely want to keep "no-default-data-layout", but not necessarily "default-data-layout".> > Would it make sense to have opt issue an error if the module and the > command-line have incompatible non-empty strings?Yes, it would, come to think of it. If llvm-gcc put a target data string, it generated code under the assumption that 'opt' would not use a different target data string. Breaking that assumption could be very bad. (Not having one at all should be safe, since that would keep opt from messing with existing pointer logic).> It would break existing tests, which currently rely on an empty > targetdata string being interpreted as historical sparc settings. > But tests can be updated; it's more important to figure out how > to make opt useful first.I was hesitating because fixing the tests and changing opt would have to be done together in one shot, or else the tests would be broken for a while. And that seemed like a daunting task. I think "no-default-data-layout" all by itself gets us where we need to be to have opt be useful in just about every case I can think of. And if production code with pointer operations works with an optimizer that targets a completely different platform and messes with it's pointer operations, it's probably very close to breaking mysteriously at any rate. So having "no-default-data-layout" be the default would make production code *more* stable and mainly break those tests, as far as I can see. How many tests are we talking, and what's the best way to attack that problem? And a force-data-layout with an error if the module has a different one... that option would find its way into my makefiles as the last step after an application is linked with the target-independent standard libraries and is ready to be optimized in a single unit, whole-program style, with optimizations for the install platform included without having to make my compiler deal with it. And if modules targeting a different platform find their way into my build process, I definitely want to bring the build to a screeching halt.
> I was hesitating because fixing the tests and changing opt would have > to be done together in one shot, or else the tests would be broken for > a while. And that seemed like a daunting task. > > I think "no-default-data-layout" all by itself gets us where we need > to be to have opt be useful in just about every case I can think of. > And if production code with pointer operations works with an optimizer > that targets a completely different platform and messes with it's > pointer operations, it's probably very close to breaking mysteriously > at any rate. So having "no-default-data-layout" be the default would > make production code *more* stable and mainly break those tests, as > far as I can see. How many tests are we talking, and what's the best > way to attack that problem?I see two approaches so far: 1. Change the tests one by one, having them use the "no-default-data-layout" flag as they're updated and checked in, then make "no-default-data-layout" the default when they're all done. 2. Change all the tests to use "default-data-layout" with the Sparc setting. Then make "no-default-data-layout" the default. Update the tests one-by-one, taking off the "default-data-layout" setting as they're updated and checked in. Remove the "default-data-layout" setting when they're all done, unless someone thinks of a use for it. If I'm going to attack this, I'll need to get the tests working on my system first. (I need to do that anyway to work on the struct-return thing on my list)
On Oct 22, 2009, at 2:22 PM, Kenneth Uildriks wrote:> On Thu, Oct 22, 2009 at 3:54 PM, Dan Gohman <gohman at apple.com> wrote: >>> Anyway, after thinking about it, it should always be safe to override >>> the Module to remove the target data pass, even if it isn't safe to >>> override the Module to substitute different target data. But I still >>> think you should at least have the option to supply target data >>> *without* overriding whatever comes in from the module. >> >> In what situations would this be useful? > > It is definitely useful to me to tell opt "don't use a target data > string unless the module has one"... then I can feed it modules built > without a target platform, or modules built with the target platform > I'm using, and it'll do the best thing in both cases and break > neither. "Use this string unless the module has a different one"... > the module would only have a different one if it was compiled by > llvm-gcc or some other front-end specifically targeting some other > platform... and modules specifically targeting different platforms > probably won't get thrown together in the same build, folder, or > process. So that may not be as useful as I thought. > > In short, I definitely want to keep "no-default-data-layout", but not > necessarily "default-data-layout".Makes sense.> >> >> Would it make sense to have opt issue an error if the module and the >> command-line have incompatible non-empty strings? > > Yes, it would, come to think of it. If llvm-gcc put a target data > string, it generated code under the assumption that 'opt' would not > use a different target data string. Breaking that assumption could be > very bad. (Not having one at all should be safe, since that would > keep opt from messing with existing pointer logic). > >> It would break existing tests, which currently rely on an empty >> targetdata string being interpreted as historical sparc settings. >> But tests can be updated; it's more important to figure out how >> to make opt useful first. > > I was hesitating because fixing the tests and changing opt would have > to be done together in one shot, or else the tests would be broken for > a while. And that seemed like a daunting task. > > I think "no-default-data-layout" all by itself gets us where we need > to be to have opt be useful in just about every case I can think of. > And if production code with pointer operations works with an optimizer > that targets a completely different platform and messes with it's > pointer operations, it's probably very close to breaking mysteriously > at any rate. So having "no-default-data-layout" be the default would > make production code *more* stable and mainly break those tests, as > far as I can see. How many tests are we talking, and what's the best > way to attack that problem?I don't know offhand, though I don't think it's an overwhelming number. I'd suggest just trying it.> > And a force-data-layout with an error if the module has a different > one... that option would find its way into my makefiles as the last > step after an application is linked with the target-independent > standard libraries and is ready to be optimized in a single unit, > whole-program style, with optimizations for the install platform > included without having to make my compiler deal with it. And if > modules targeting a different platform find their way into my build > process, I definitely want to bring the build to a screeching halt.Ok. I don't currently have a need for this functionality so I'll leave it up to you. The main counter-intuitive thing for me is any option which gets silently ignored when it conflicts with what's in the module. Dan