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>
On Dec 5, 2013, at 3:19 PM, Dan Gohman <dan433584 at gmail.com> wrote:> > > > 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.We currently have something that looks like: IR Transform (links with) -> TargetTransformInfo (dynamic call) -> X86TTI (links with) -> X86Subtarget I was thinking of directly replacing X86Subtarget as such: IR Transform (links with) -> TargetTransformInfo (dynamic call) -> X86TTI (links with) -> TargetMachine (provides) -> CGContext So CGContext could still live inside libTarget. CGContext would be initialized for a Function based on the attributes and the information already in its container, TargetMachine. It sounds like you would be making CGContext part of libCore instead, like this: IR Transform (links with) -> LLVMContext (provides) -> CGContext CGContext would still need to be initialized with a TargetMachine, which could happen whenever the TargetMachine and Function are both available. I'm fine with that as long as - no one objects to putting CGContext in libCore - our goal is to eventually replace TargetTransformInfo with CGContext (maybe renamed) I think the issue here is whether we want to continue use an AnalysisGroup to vend target info, or rather to have passes get what they need from LLVMContext. I find the AnalysisGroup thing too subtle and prone to driver bugs, but haven't fully worked out an alternate design. Maybe something as simple as this would work: LLVMContext -> TargetContext (initialized when TargetMachine is available) -> SubtargetContext (initialized when Function attributes are available)> > > 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.I wouldn't expect you to address this. As long we're not making it difficult to do. Ideally we would have before and after target information passes, but that's stricter than some would like. I think it would be really horrible though to have canonicalization passes depend on cpu features. So we should make it explicit which passes do and don't depend on subtarget. Since cpu features change from function to function, this is naturally tied to initialization of CGContext. -Andy -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131205/629d28f5/attachment.html>
On Thu, Dec 5, 2013 at 4:16 PM, Andrew Trick <atrick at apple.com> wrote:> We currently have something that looks like: > > IR Transform > (links with) -> TargetTransformInfo > (dynamic call) -> X86TTI > (links with) -> X86Subtarget > > I was thinking of directly replacing X86Subtarget as such: > > IR Transform > (links with) -> TargetTransformInfo > (dynamic call) -> X86TTI > (links with) -> TargetMachine > (provides) -> CGContext > > So CGContext could still live inside libTarget. CGContext would be > initialized for a Function based on the attributes and the information > already in its container, TargetMachine. > > It sounds like you would be making CGContext part of libCore instead, like > this: > > IR Transform > (links with) -> LLVMContext > (provides) -> CGContext > > CGContext would still need to be initialized with a TargetMachine, which > could happen whenever the TargetMachine and Function are both available. > > I'm fine with that as long as > - no one objects to putting CGContext in libCore > - our goal is to eventually replace TargetTransformInfo with CGContext > (maybe renamed) > > I think the issue here is whether we want to continue use an AnalysisGroup > to vend target info, or rather to have passes get what they need from > LLVMContext. I find the AnalysisGroup thing too subtle and prone to driver > bugs, but haven't fully worked out an alternate design. >I'm not actually a fan of the analysis group hack, and there are better ways to slice this, but I don't really like the direction of putting the CGContext into the LLVMContext. There are still very viable uses of LLVM which have *no* concept of a target outside of the LLVM IR itself. I think we should preserve this abstraction boundary. I think that means there needs to be an *abstract* interface between the IR layer and the target/codegen layer with a dynamic call across that boundary. So this makes sense to me: LLVMContext> -> TargetContext (initialized when TargetMachine is available) > -> SubtargetContext (initialized when Function attributes are > available) >Only if TargetContext and SubtargetContext are abstract interfaces with trivial implementations available for cases where there is no target (in the libTarget sense). I don't really think that's what you're aiming for, so I would advocate for the MachineModuleInfo or the TargetMachine being the provider of the CGContext or the TargetContext and SubtargetContext. I like the TargetMachine providing it personally, but I don't feel strongly here. I also think that we could start with the MachineModuleInfo providing it and refactor that in a subsequent patch. FWIW, I have a plan to remove the crazy analysis group stuff in the new pass manager. My hope is that we have a simple analysis which can be configured with dynamic calls into a TargetMachine (or some TargetMachine-wrapping interface). It will be very explicit with none of the analysis group magic. It will just preserve the dynamic interface boundary between the the IR layer and the target layer. -Chandler -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131205/b3f3006f/attachment.html>
Trying to bubble way back to the top, Andy, do you think there is anything else that needs to be done here before it can go in? I feel like most of our discussion centered around future work, and I'd like to unblock the immediate work to support subtarget-specific code generation. On Thu, Dec 5, 2013 at 6:19 PM, Dan Gohman <dan433584 at gmail.com> wrote:> > > > 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 > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140106/184ff912/attachment.html>
On Jan 6, 2014, at 10:10 AM, Chandler Carruth <chandlerc at google.com> wrote:> Trying to bubble way back to the top, Andy, do you think there is anything else that needs to be done here before it can go in? I feel like most of our discussion centered around future work, and I'd like to unblock the immediate work to support subtarget-specific code generation.Do we have a solution that can work with TTI? I don’t think there was any objection to putting CGContext in TargetMachine, except that we have too many layers of abstraction, which you said will be cleaned up eventually: IR Transform (links with) -> TargetTransformInfo (dynamic call) -> X86TTI (links with) -> TargetMachine (provides) -> CGContext -Andy> > > On Thu, Dec 5, 2013 at 6:19 PM, Dan Gohman <dan433584 at gmail.com> wrote: > > > > 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 > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140106/8dd4b3b2/attachment.html>