On Wed, 2010-05-05 at 13:22 -0700, Nathan Jeffords wrote:> > The important point here is that the COFF MCSection needs to > have the right level of semantic information. In fact, > MCSection is the place that I'd start for COFF bringup. > > OK, I see that now. The current isolation > between TargetLoweringObjectFile -> MCStreamer -> MCObjectWriter has > proven somewhat problematic, mostly due to my lack of understanding. > I guess MCSectionXXX was meant to provide communication between them. > Should the same be true of MCSymbol, and their data counterparts?I'm enclosing my patch for reforming MCSectionCOFF to match the implementation strategy of the other two MCSection classes. You may find it useful as a starting point. It seems to be complete and correct, and worked for what I tried with it, but I didn't find time to test it fully (e.g., by bootstrapping clang under Cygwin). Cheers, -Peter- -------------- next part -------------- A non-text attachment was scrubbed... Name: mcsectioncoff.diff Type: text/x-patch Size: 24420 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100506/9b09aebb/attachment.bin>
Thanks! Funny, I was just preparing a patch to submit for my changes to MCSectionCOFF. My changes look to be fairly independent of yours, my change was to deal with COMDAT's. I had dealt with the characteristics flags in the object writer, but I like this. If you don't mind I would like to merge my changes into this patch and submit it. I was just pondering how to deal with the PrintSwitchToSection function without needing the IsDirective flag. On Thu, May 6, 2010 at 11:12 PM, Peter S. Housel <housel at acm.org> wrote:> On Wed, 2010-05-05 at 13:22 -0700, Nathan Jeffords wrote: > > > > > The important point here is that the COFF MCSection needs to > > have the right level of semantic information. In fact, > > MCSection is the place that I'd start for COFF bringup. > > > > OK, I see that now. The current isolation > > between TargetLoweringObjectFile -> MCStreamer -> MCObjectWriter has > > proven somewhat problematic, mostly due to my lack of understanding. > > I guess MCSectionXXX was meant to provide communication between them. > > Should the same be true of MCSymbol, and their data counterparts? > > I'm enclosing my patch for reforming MCSectionCOFF to match the > implementation strategy of the other two MCSection classes. You may find > it useful as a starting point. It seems to be complete and correct, and > worked for what I tried with it, but I didn't find time to test it fully > (e.g., by bootstrapping clang under Cygwin). > > Cheers, > -Peter- > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100506/169d6339/attachment.html>
On May 6, 2010, at 11:22 PM, Nathan Jeffords wrote:> Thanks! Funny, I was just preparing a patch to submit for my changes to MCSectionCOFF. My changes look to be fairly independent of yours, my change was to deal with COMDAT's. I had dealt with the characteristics flags in the object writer, but I like this. If you don't mind I would like to merge my changes into this patch and submit it. I was just pondering how to deal with the PrintSwitchToSection function without needing the IsDirective flag.I prefer to merge in small independent patches as they are built. Please review Peter's patch (since you know COFF :). I'll take a look tomorrow and apply it if you think it is forward progress, and if there aren't other issues. Thanks! -Chris> > On Thu, May 6, 2010 at 11:12 PM, Peter S. Housel <housel at acm.org> wrote: > On Wed, 2010-05-05 at 13:22 -0700, Nathan Jeffords wrote: > > > > > The important point here is that the COFF MCSection needs to > > have the right level of semantic information. In fact, > > MCSection is the place that I'd start for COFF bringup. > > > > OK, I see that now. The current isolation > > between TargetLoweringObjectFile -> MCStreamer -> MCObjectWriter has > > proven somewhat problematic, mostly due to my lack of understanding. > > I guess MCSectionXXX was meant to provide communication between them. > > Should the same be true of MCSymbol, and their data counterparts? > > I'm enclosing my patch for reforming MCSectionCOFF to match the > implementation strategy of the other two MCSection classes. You may find > it useful as a starting point. It seems to be complete and correct, and > worked for what I tried with it, but I didn't find time to test it fully > (e.g., by bootstrapping clang under Cygwin). > > Cheers, > -Peter- > > > _______________________________________________ > 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/20100507/b7c96a0f/attachment.html>
On May 6, 2010, at 11:12 PM, Peter S. Housel wrote:> On Wed, 2010-05-05 at 13:22 -0700, Nathan Jeffords wrote: > >> >> The important point here is that the COFF MCSection needs to >> have the right level of semantic information. In fact, >> MCSection is the place that I'd start for COFF bringup. >> >> OK, I see that now. The current isolation >> between TargetLoweringObjectFile -> MCStreamer -> MCObjectWriter has >> proven somewhat problematic, mostly due to my lack of understanding. >> I guess MCSectionXXX was meant to provide communication between them. >> Should the same be true of MCSymbol, and their data counterparts? > > I'm enclosing my patch for reforming MCSectionCOFF to match the > implementation strategy of the other two MCSection classes. You may find > it useful as a starting point. It seems to be complete and correct, and > worked for what I tried with it, but I didn't find time to test it fully > (e.g., by bootstrapping clang under Cygwin).Looks really great to me, applied in r103267, thanks! One thing: +++ include/llvm/CodeGen/TargetLoweringObjectFileImpl.h (working copy) @@ -161,13 +161,15 @@ ... + virtual const MCSection *getDrectveSection() const { return DrectveSection; } This shouldn't need to be virtual? -Chris