On Jul 16, 2009, at 12:53 AM, Aaron Gray wrote:> Chris, > > The basic idea of using templates inconjunction with inlining is for > efficiency. > > 6,500 virtual calls outputting bytes out of 10000 calls, and the > rest 1,750 being words to output 10,000 of code does not entice me > to use virtual calls.I understand that you say that, but I can't bring myself to care at this point. Have you thought about how many cycles are already used to produce the instructions that lead to the emission of those 10K bytes? The total percentage of time spent doing these virtual calls will be tiny compared to the total time to generate the code. If you switch to using virtual functions, get the code working, and we *measure* a performance problem, then we can fix it. There are much better ways to do this than templating the whole code emitter.> Whats Daniels approach, does he have any online documentation or > code, do you have an email address so I may talk to him.Take a look at how asmprinters work in include/llvm/Target/ TargetRegistry.h . If you have specific questions, llvmdev is a great place to ask them. -Chris> > Aaron > > 2009/7/16 Chris Lattner <clattner at apple.com> > On Jul 15, 2009, at 9:01 AM, Aaron Gray wrote: > Chris, > > If you/we do not like this code, then the alternatives are :- > 1) Leave as is, which I would not suggest. > 2) Revert to using MachineCodeEmitter like mechanics with virtual > extend() method to allow rebuffering with ObjectCodeEmitter > providing the memory management. > 3) Don't really know of any other alternatives :) > > 2 maybe the best compromise option. Its easy to code, removes > templating from the CodeEmitters, and is virtually transparent to > our other DOE work. The only thing it does not write via a > BinaryObject object, so we loose that functionality. > > Hi Aaron, > > I'm sorry for not getting back to you sooner. I work fairly LIFO > and the craziness that has happened since your emails have > distracted me. > > One problem with this patch is that (for example) > X86TargetMachine.cpp refers to the "createX86ObjectCodeEmitterPass" > symbol, which forces the code emitter to object code emitter to be > linked into the X86 target module. There is still no way to create > a JIT without two copies of the code emitter template. I believe > that aligning this work with Daniels work to make the targets more > modular would be straight forward. Are you willing to do this? > > This patch keeps around the templates, which I really don't like. > However, this is better than what is in mainline, so it seems like a > reasonable step to me if you really really want to do this. > However, I still don't understand why you're unwilling to make the > CodeEmitter be a virtual base class instead of a template! > > -Chris >-------------- next part -------------- An HTML attachment was scrubbed... URL: <lists.llvm.org/pipermail/llvm-dev/attachments/20090716/20f5f484/attachment.html>
2009/7/16 Chris Lattner <clattner at apple.com>> > On Jul 16, 2009, at 12:53 AM, Aaron Gray wrote: > > Chris, > > The basic idea of using templates inconjunction with inlining is for > efficiency. > > 6,500 virtual calls outputting bytes out of 10000 calls, and the rest 1,750 > being words to output 10,000 of code does not entice me to use virtual > calls. > > > I understand that you say that, but I can't bring myself to care at this > point. Have you thought about how many cycles are already used to produce > the instructions that lead to the emission of those 10K bytes? The total > percentage of time spent doing these virtual calls will be tiny compared to > the total time to generate the code. > > If you switch to using virtual functions, get the code working, and we > *measure* a performance problem, then we can fix it. There are much better > ways to do this than templating the whole code emitter. >That means JIT code also has the virtual function overhead too, this will slow down existing JIT code. Templates are already there and they work and they do not take up _too_ much memory.> > Whats Daniels approach, does he have any online documentation or code, do > you have an email address so I may talk to him. > > > Take a look at how asmprinters work in include/llvm/Target/TargetRegistry.h > . If you have specific questions, llvmdev is a great place to ask them. >Okay I will take a look. Aaron> > -Chris > > > > Aaron > > 2009/7/16 Chris Lattner <clattner at apple.com> > >> On Jul 15, 2009, at 9:01 AM, Aaron Gray wrote: >> >>> Chris, >>> >>> If you/we do not like this code, then the alternatives are :- >>> 1) Leave as is, which I would not suggest. >>> 2) Revert to using MachineCodeEmitter like mechanics with virtual >>> extend() method to allow rebuffering with ObjectCodeEmitter providing the >>> memory management. >>> 3) Don't really know of any other alternatives :) >>> >>> 2 maybe the best compromise option. Its easy to code, removes templating >>> from the CodeEmitters, and is virtually transparent to our other DOE work. >>> The only thing it does not write via a BinaryObject object, so we loose that >>> functionality. >>> >> >> Hi Aaron, >> >> I'm sorry for not getting back to you sooner. I work fairly LIFO and the >> craziness that has happened since your emails have distracted me. >> >> One problem with this patch is that (for example) X86TargetMachine.cpp >> refers to the "createX86ObjectCodeEmitterPass" symbol, which forces the code >> emitter to object code emitter to be linked into the X86 target module. >> There is still no way to create a JIT without two copies of the code >> emitter template. I believe that aligning this work with Daniels work to >> make the targets more modular would be straight forward. Are you willing to >> do this? >> >> This patch keeps around the templates, which I really don't like. >> However, this is better than what is in mainline, so it seems like a >> reasonable step to me if you really really want to do this. However, I >> still don't understand why you're unwilling to make the CodeEmitter be a >> virtual base class instead of a template! >> >> -Chris >> > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <lists.llvm.org/pipermail/llvm-dev/attachments/20090716/6145cead/attachment.html>
On Thu, Jul 16, 2009 at 10:04 AM, Aaron Gray<aaronngray.lists at googlemail.com> wrote:> 2009/7/16 Chris Lattner <clattner at apple.com> >> Whats Daniels approach, does he have any online documentation or code, do >> you have an email address so I may talk to him. >> >> Take a look at how asmprinters work >> in include/llvm/Target/TargetRegistry.h . If you have specific questions, >> llvmdev is a great place to ask them. > > > > Okay I will take a look.I don't have any documentation yet other than the doxygen comments (some will be added at least before 2.6), but the basic idea is that there is one global Target instance per target, and targets register optional components via initialization functions (which can be called via static constructors, or explicitly by the client). Clients of the targets simple request a Target, which will always be linked in, and look to see if the optional functionality is present (i.e. was linked in). - Daniel> Aaron > >> >> -Chris >> >> >> Aaron >> >> 2009/7/16 Chris Lattner <clattner at apple.com> >>> >>> On Jul 15, 2009, at 9:01 AM, Aaron Gray wrote: >>>> >>>> Chris, >>>> >>>> If you/we do not like this code, then the alternatives are :- >>>> 1) Leave as is, which I would not suggest. >>>> 2) Revert to using MachineCodeEmitter like mechanics with virtual >>>> extend() method to allow rebuffering with ObjectCodeEmitter providing the >>>> memory management. >>>> 3) Don't really know of any other alternatives :) >>>> >>>> 2 maybe the best compromise option. Its easy to code, removes templating >>>> from the CodeEmitters, and is virtually transparent to our other DOE work. >>>> The only thing it does not write via a BinaryObject object, so we loose that >>>> functionality. >>> >>> Hi Aaron, >>> >>> I'm sorry for not getting back to you sooner. I work fairly LIFO and the >>> craziness that has happened since your emails have distracted me. >>> >>> One problem with this patch is that (for example) X86TargetMachine.cpp >>> refers to the "createX86ObjectCodeEmitterPass" symbol, which forces the code >>> emitter to object code emitter to be linked into the X86 target module. >>> There is still no way to create a JIT without two copies of the code >>> emitter template. I believe that aligning this work with Daniels work to >>> make the targets more modular would be straight forward. Are you willing to >>> do this? >>> >>> This patch keeps around the templates, which I really don't like. >>> However, this is better than what is in mainline, so it seems like a >>> reasonable step to me if you really really want to do this. However, I >>> still don't understand why you're unwilling to make the CodeEmitter be a >>> virtual base class instead of a template! >>> >>> -Chris >> >> > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu llvm.cs.uiuc.edu > lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >
On Jul 16, 2009, at 10:04 AM, Aaron Gray wrote:> > I understand that you say that, but I can't bring myself to care at > this point. Have you thought about how many cycles are already used > to produce the instructions that lead to the emission of those 10K > bytes? The total percentage of time spent doing these virtual calls > will be tiny compared to the total time to generate the code. > > If you switch to using virtual functions, get the code working, and > we *measure* a performance problem, then we can fix it. There are > much better ways to do this than templating the whole code emitter. > > That means JIT code also has the virtual function overhead too, this > will slow down existing JIT code. Templates are already there and > they work and they do not take up _too_ much memory.As I said before, if you are compelled to, feel free to continue with your approach of premature optimization. I will fix it later. -Chris
Seemingly Similar Threads
- [LLVMdev] [patch] CodeEmitter Memory Foot Reduction
- [LLVMdev] [patch] CodeEmitter Memory Foot Reduction
- [LLVMdev] [patch] CodeEmitter Memory Foot Reduction
- [LLVMdev] [patch] CodeEmitter Memory Foot Reduction
- [LLVMdev] [patch] CodeEmitter Memory Foot Reduction