On 1 December 2011 17:15, Chris Lattner <clattner at apple.com> wrote:> > On Dec 1, 2011, at 5:09 PM, Nick Lewycky wrote: > >> I'm running LLVM under threadsanitizer >> (http://code.google.com/p/data-race-test/) to find and remove races in >> a larger program that uses LLVM as a library. One of the things that I >> found is that the TargetOptions are all global; you could create a >> TargetMachine targeting ARM and X86 in two threads, but they both have >> to share the same FloatABIType setting. >> >> This is silly, and I plan to fix it by moving all the globals in >> TargetOptions into getters and setters on TargetMachine. This means >> that the TargetMachine object is going to get a bit bigger to >> accommodate, but at least two users of llvm linked into one program >> won't step on each other. > > Sounds great. > >> The existing command-line options will move >> from TargetMachine into llc which will use the same setters. This has >> the side-effect that "clang -mllvm" won't work for them any more. > > Sounds fine.Thanks Chris!> Instead of adding a bunch of instance variables (+ getters/setters) into TargetMachine, why not make TargetOptions be a class, and have TM contain an instance of it?That works too, it makes little difference to me. One reason is that most references to these globals are inside classes that derive from TargetMachine so I wouldn't need to touch those lines. The second reason is that I'm going to turn "bool Foo" into "unsigned Foo : 1" and would like that to pack in with the 6 variables that are currently members of the TargetMachine. How about I just move the 6 existing flags into TargetOptions and even remove their getters/setters. I think that'll be cleanest overall. Nick
On Dec 1, 2011, at 5:23 PM, Nick Lewycky wrote:>> Instead of adding a bunch of instance variables (+ getters/setters) into TargetMachine, why not make TargetOptions be a class, and have TM contain an instance of it? > > That works too, it makes little difference to me. One reason is that > most references to these globals are inside classes that derive from > TargetMachine so I wouldn't need to touch those lines. The second > reason is that I'm going to turn "bool Foo" into "unsigned Foo : 1" > and would like that to pack in with the 6 variables that are currently > members of the TargetMachine. > > How about I just move the 6 existing flags into TargetOptions and even > remove their getters/setters. I think that'll be cleanest overall.I'd really prefer these to be in a separate class: TM is just too huge and full of random things already. I'm not concerned about the actual sizeof(TargetMachine), so reusing a few bits in it shouldn't matter. -Chris
Chris Lattner wrote:> On Dec 1, 2011, at 5:23 PM, Nick Lewycky wrote: >>> Instead of adding a bunch of instance variables (+ getters/setters) into TargetMachine, why not make TargetOptions be a class, and have TM contain an instance of it? >> >> That works too, it makes little difference to me. One reason is that >> most references to these globals are inside classes that derive from >> TargetMachine so I wouldn't need to touch those lines. The second >> reason is that I'm going to turn "bool Foo" into "unsigned Foo : 1" >> and would like that to pack in with the 6 variables that are currently >> members of the TargetMachine. >> >> How about I just move the 6 existing flags into TargetOptions and even >> remove their getters/setters. I think that'll be cleanest overall. > > I'd really prefer these to be in a separate class: TM is just too huge and full of random things already. I'm not concerned about the actual sizeof(TargetMachine), so reusing a few bits in it shouldn't matter.Sounds good, committed in r145714 and a number of followup patches for the frontends. Unfortunately there's two failing buildbots left, both building llvm-gcc on ARM: http://lab.llvm.org:8011/builders/llvm-gcc-i686-pc-linux-gnu-cross-arm-eabi-soft-float/ http://lab.llvm.org:8011/builders/llvm-gcc-i686-pc-linux-gnu-cross-arm-eabi-hard-float/ Galina, those are yours. I don't understand why the buildbots are failing (where are they putting -soft-float and -float-abi on cc1's command line?) but I also don't understand why we're still building llvm-gcc. I really only want to bring this up because I'm worried that the same bug might be shared with dragonegg. Is this something you can investigate? Nick