Hello llvm-dev, Following up on the "CodeGen Context" discussion that was started, attached is patch which implements a pretty minimal skeleton of a CGContext implementation. The goal is to allow the newly added subtarget attributes on functions to be made available to codegen so that codegen can actually switch subtargets within a Module. Comments and questions are invited herewith. There has been some disagreement as to what to name this class. I have stuck with "CGContext" for now, though I'm absolutely not attached to that name. It demonstrates where the CGContext state lives, how code can access it (in X86ISelLowering.cpp), how it reads the attributes from the IR, and how it gets cached. One thing that it doesn't yet do is hook up to the target-specific subtarget feature interpretation. For example, it would need to hook up to the x86 subtarget code which knows that, for example, sse4.2 implies sse4.1, and so on. However, I'm sending this out now so that the other parts of the patch can get some feedback. Thanks, Dan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131125/97da959c/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: cgcontext.patch Type: text/x-diff Size: 7035 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131125/97da959c/attachment.patch>
On Nov 25, 2013, at 4:40 PM, Dan Gohman <dan433584 at gmail.com> wrote:> Hello llvm-dev, > > Following up on the "CodeGen Context" discussion that was started, attached is patch which implements a pretty minimal skeleton of a CGContext implementation. The goal is to allow the newly added subtarget attributes on functions to be made available to codegen so that codegen can actually switch subtargets within a Module. > > Comments and questions are invited herewith. > > There has been some disagreement as to what to name this class. I have stuck with "CGContext" for now, though I'm absolutely not attached to that name. > > It demonstrates where the CGContext state lives, how code can access it (in X86ISelLowering.cpp), how it reads the attributes from the IR, and how it gets cached. > > One thing that it doesn't yet do is hook up to the target-specific subtarget feature interpretation. For example, it would need to hook up to the x86 subtarget code which knows that, for example, sse4.2 implies sse4.1, and so on. However, I'm sending this out now so that the other parts of the patch can get some feedback. >Hi Dan, Thanks for working on this! I like the direction this is going. One thing though, the function attributes no longer have “target-cpu” and “target-features” in them. I opted to separate out all of the different flags into separate attributes, instead of relying upon a single feature string. So the caching key would need to be modified here. Or do you think we should go back to adding back those attributes? One thing to be addressed is when one function has attributes but the next function lacks them. Some type of default value needs to be assumed for the function lacking an attribute, and the context cached accordingly. -bw
On Tue, Nov 26, 2013 at 6:31 AM, Bill Wendling <isanbard at gmail.com> wrote:> On Nov 25, 2013, at 4:40 PM, Dan Gohman <dan433584 at gmail.com> wrote: > > > Hello llvm-dev, > > > > Following up on the "CodeGen Context" discussion that was started, > attached is patch which implements a pretty minimal skeleton of a CGContext > implementation. The goal is to allow the newly added subtarget attributes > on functions to be made available to codegen so that codegen can actually > switch subtargets within a Module. > > > > Comments and questions are invited herewith. > > > > There has been some disagreement as to what to name this class. I have > stuck with "CGContext" for now, though I'm absolutely not attached to that > name. > > > > It demonstrates where the CGContext state lives, how code can access it > (in X86ISelLowering.cpp), how it reads the attributes from the IR, and how > it gets cached. > > > > One thing that it doesn't yet do is hook up to the target-specific > subtarget feature interpretation. For example, it would need to hook up to > the x86 subtarget code which knows that, for example, sse4.2 implies > sse4.1, and so on. However, I'm sending this out now so that the other > parts of the patch can get some feedback. > > > Hi Dan, >Hi Bill,> Thanks for working on this! I like the direction this is going. One thing > though, the function attributes no longer have “target-cpu” and > “target-features” in them. I opted to separate out all of the different > flags into separate attributes, instead of relying upon a single feature > string. So the caching key would need to be modified here. Or do you think > we should go back to adding back those attributes? >One of the advantages os using a single target-features string is that it's easy to extend; there's no need for the CGContext code to know which attributes it should care about and which it should ignore, unless we introduce a naming convention. Alternatively, we could just have it read *all* the attributes all the time. What do you think?> > One thing to be addressed is when one function has attributes but the next > function lacks them. Some type of default value needs to be assumed for the > function lacking an attribute, and the context cached accordingly.Yes, that's a good point. I'll update my patch to include code to handle that case. Dan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131126/4855519f/attachment.html>
On Nov 25, 2013, at 4:40 PM, Dan Gohman <dan433584 at gmail.com> wrote:> Hello llvm-dev, > > Following up on the "CodeGen Context" discussion that was started, attached is patch which implements a pretty minimal skeleton of a CGContext implementation. The goal is to allow the newly added subtarget attributes on functions to be made available to codegen so that codegen can actually switch subtargets within a Module. > > Comments and questions are invited herewith. > > There has been some disagreement as to what to name this class. I have stuck with "CGContext" for now, though I'm absolutely not attached to that name. > > It demonstrates where the CGContext state lives, how code can access it (in X86ISelLowering.cpp), how it reads the attributes from the IR, and how it gets cached. > > One thing that it doesn't yet do is hook up to the target-specific subtarget feature interpretation. For example, it would need to hook up to the x86 subtarget code which knows that, for example, sse4.2 implies sse4.1, and so on. However, I'm sending this out now so that the other parts of the patch can get some feedback.Hi Dan, This basic idea makes sense to me. It is consistent with my understanding of Bill's proposal. What I don't understand is why CGContext is tied to to CodeGen objects and initialized at ISelLowering. We should be able to get a CGContext from a TargetMachine by providing a Function. IR passes like the LoopVectorizer and LSR also require subtarget information. This was formalized by TargetTransformInfo. Now any pass using TargetTransformInfo needs to respect the current Function attributes, which means reinitializing subtarget info. I also think that IR passes should use the same interface as SD/MI passes to query codegen modes, rather than directly querying Function attributes. For that reason, they should also use CGContext. It should be possible to override those modes with command line options, but I don't think we should have ad-hoc overrides buried within passes. I think it would be cleanest if the IR pass pipeline had an explicit point at which the subtarget gets initialized--say a SubtargetInfo pass. IR passes that run early can still get architecture defaults from TargetTransformInfo, but can't make any subtarget specific queries. IR passes that run after this point should be able to make CGContext queries, either directly or via TargetTransformInfo (I'm not sure there is any real advantage in treating TargetTransformInfo as an analysis). -Andy> > Thanks, > > Dan > > <cgcontext.patch>_______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Mon, Dec 2, 2013 at 4:25 PM, Andrew Trick <atrick at apple.com> wrote:> > On Nov 25, 2013, at 4:40 PM, Dan Gohman <dan433584 at gmail.com> wrote: > > > Hello llvm-dev, > > > > Following up on the "CodeGen Context" discussion that was started, > attached is patch which implements a pretty minimal skeleton of a CGContext > implementation. The goal is to allow the newly added subtarget attributes > on functions to be made available to codegen so that codegen can actually > switch subtargets within a Module. > > > > Comments and questions are invited herewith. > > > > There has been some disagreement as to what to name this class. I have > stuck with "CGContext" for now, though I'm absolutely not attached to that > name. > > > > It demonstrates where the CGContext state lives, how code can access it > (in X86ISelLowering.cpp), how it reads the attributes from the IR, and how > it gets cached. > > > > One thing that it doesn't yet do is hook up to the target-specific > subtarget feature interpretation. For example, it would need to hook up to > the x86 subtarget code which knows that, for example, sse4.2 implies > sse4.1, and so on. However, I'm sending this out now so that the other > parts of the patch can get some feedback. > > Hi Dan, > > This basic idea makes sense to me. It is consistent with my understanding > of Bill's proposal. What I don't understand is why CGContext is tied to to > CodeGen objects and initialized at ISelLowering. We should be able to get a > CGContext from a TargetMachine by providing a Function. >> IR passes like the LoopVectorizer and LSR also require subtarget > information. This was formalized by TargetTransformInfo. Now any pass using > TargetTransformInfo needs to respect the current Function attributes, which > means reinitializing subtarget info. I also think that IR passes should use > the same interface as SD/MI passes to query codegen modes, rather than > directly querying Function attributes. For that reason, they should also > use CGContext. It should be possible to override those modes with command > line options, but I don't think we should have ad-hoc overrides buried > within passes. >In my current patch, the CGContext instances are kept within MachineModuleInfo instance. You're right that this is a Codegen-oriented class. What would you think about having the CGContext instances kept in the LLVMContext? This would also allow them to be cached in the same way, and it would make them available to any part of the middle or backend that wanted them.> > I think it would be cleanest if the IR pass pipeline had an explicit point > at which the subtarget gets initialized--say a SubtargetInfo pass. IR > passes that run early can still get architecture defaults from > TargetTransformInfo, but can't make any subtarget specific queries. IR > passes that run after this point should be able to make CGContext queries, > either directly or via TargetTransformInfo (I'm not sure there is any real > advantage in treating TargetTransformInfo as an analysis). >Dividing the optimization pipeline into "before" and "after" subtarget information is a bit beyond the scope of what I'm looking to do here. If CGContext is moved to LLVMContext (and perhaps renamed), then it can be available to any pass, and the decision of when to introduce subtarget information can be made at a higher level. Dan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131205/c49d60b8/attachment.html>