On Tuesday 03 July 2007 00:44, Chris Lattner wrote:> On Mon, 2 Jul 2007, David A. Greene wrote: > >>> - Changing the API > >>> a) Template it to take two iterators. This causes code size bloat. > > > > This seems like the right solution to me. Unless llvm is running on > > extremely limited memory embedded systems, the extra code space > > shouldn't be an issue. > > Code size is always an issue.To a greater or lesser degree. Context matters.> Here's a different suggestion that cloning all the code. Instead of doing > that, why not add a new (templated) CallInst ctor, one which is very > trivial. Then you could put the conditional code in it (to detect an > empty range) and call into the non-inline stuff we already have. > > It should be fine to only support random access iterators, so you could > call into the existing stuff that takes a base + size.That's pretty close to what I was thinking. I think it's a good solution. -Dave
On Tue, 3 Jul 2007, David Greene wrote:>>> This seems like the right solution to me. Unless llvm is running on >>> extremely limited memory embedded systems, the extra code space >>> shouldn't be an issue. >> >> Code size is always an issue. > > To a greater or lesser degree. Context matters.You're right of course, let me rephrase what I meant. These are changes to VMCore. Any bloat to VMCore will affect anything that uses the LLVM IR. As such, bloat can prevent LLVM from being used by various things that it might otherwise be suitable for. We did quite a bit of work to reduce template bloat to reduce codesize for the opengl work we did, for example. Code size does matter :) -Chris -- http://nondot.org/sabre/ http://llvm.org/
On Tuesday 03 July 2007 10:58, David Greene wrote:> > Here's a different suggestion that cloning all the code. Instead of > > doing that, why not add a new (templated) CallInst ctor, one which is > > very trivial. Then you could put the conditional code in it (to detect > > an empty range) and call into the non-inline stuff we already have. > > > > It should be fine to only support random access iterators, so you could > > call into the existing stuff that takes a base + size. > > That's pretty close to what I was thinking. I think it's a good solution.I'm happy to go do this but I'm wondering if we should include some concept-checking code or something equivalent to ensure that we're passed random access iterators. What do you all think? -Dave
On Tuesday 03 July 2007 15:25, David Greene wrote:> I'm happy to go do this but I'm wondering if we should include some > concept-checking code or something equivalent to ensure that we're > passed random access iterators.I've got a patch and it turns out I didn't need anything as fancy as concepts. A simple call out to another init() routine that takes an iterator_category object is sufficient. I'll test and get this in ASAP. -Dave
On Tue, 3 Jul 2007, David Greene wrote:>> That's pretty close to what I was thinking. I think it's a good solution. > > I'm happy to go do this but I'm wondering if we should include some > concept-checking code or something equivalent to ensure that we're > passed random access iterators. > > What do you all think?Please keep it simple :). We can deal with ugly error messages if it explodes. -Chris -- http://nondot.org/sabre/ http://llvm.org/