[Diffusion] rL237954: Resubmit r237708 (MIR Serialization: print and parse LLVM IR using MIR format).

[Alex L]

I've decided that the approach where MIR printing is in lib/CodeGen and the
parsing is in lib/CodeGen/MIRParser will work
well for now - the circular dependency will no longer exist as CodeGen
won't depend on MIRParser.
The MIR serialization printing and parsing can also be moved when the rest
of the Machine IR is moved out of CodeGen,
so I think it's best to commit it before that so that I can work on both in
parallel.

I've attached an updated patch (mirSerialization.patch) that has the
reverted commit with the MIRParser library reorganization.
I've also attached a second patch that shows the reorganization diff
between the reverted commit and the updated patch.

Cheers,
Alex



2015-05-22 15:31 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:

>
> > On 2015-May-22, at 15:15, Alex L <arphaman at gmail.com> wrote:
> >
> >
> >
> > 2015-05-22 15:09 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com
> >:
> >
> > > On 2015-May-22, at 14:11, Alex L <arphaman at gmail.com> wrote:
> > >
> > >
> > >
> > > 2015-05-22 11:00 GMT-07:00 Duncan P. N. Exon Smith <
> dexonsmith at apple.com>:
> > > Why not move the machine IR state into lib/CodeGen/MIR?  Then it
> > > won't depend on lib/CodeGen.
> > >
> > > While moving the cpp files for MachineFunction, MachineBasicBlock,
> etc. to MIR
> > > would be easy, moving the headers will require changes to all the
> targets. At the same time the headers
> > > can stay under CodeGen just like the header file for AsmPrinter, but I
> would prefer either to move
> > > everything or move nothing at all. I attached a patch below that moves
> the necessary files from CodeGen
> > > to a new library called MIR, but I don't think this kind of change
> will be accepted.
> >
> > Why not?  This patch makes sense to me.  I think it's a good
> > cleanup *anyway*.
> >
> > (Deserves its own thread though, since as you indicate it's a
> > fairly invasive change.)
> >
> > I'm just worried about the timeframe. I'll bring it up in on llvm-dev
> with a short proposal and a patch and we'll see
> > what happens.
>
> In the short term, I'm fine with:
>
>   - lib/CodeGen: machine IR + the rest of CodeGen
>   - lib/CodeGen/MIRParser: machine IR parser
>
> as long as you keep pushing for better organization in parallel.
> I don't imagine you'll get any resistance, though.
>
> > > I think the best approach here would be to move everything from
> CodeGen/MIR to CodeGen so that
> > > the serialization will be inside of a CodeGen library. The approach
> that I outlined in my previous email where
> > > only the MIRParser is a separate library should also work.
> >
> > Having a separate library for the parser sounds like a good idea
> > regardless.  This makes sense to me:
> >
> >   - lib/MIR: machine IR
> >   - lib/MIR/MIRParser: machine IR parser
> >
> > >
> > > Cheers,
> > > Alex.
> > >
> > >
> > > > On 2015 May 22, at 09:38, Alex L <arphaman at gmail.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > I'm not sure if it would possible to break this circular dependency.
> > > >
> > > > LLVMMIR has to depend on LLVMCodeGen as it needs to access all the
> machine IR state.
> > > > But at the same time LLVMCodeGen has to connect to LLVMMIR to create
> a printing pass and
> > > > then later to parse the machine functions, as llc does all the pass
> setup with LLVMCodeGen.
> > > >
> > > > If this dependency is a problem maybe LLVMMIR should be a part of
> LLVMCodeGen?
> > > >
> > > > Alex.
> > > >
> > > > 2015-05-22 7:48 GMT-07:00 NAKAMURA Takumi <geek4civic at gmail.com>:
> > > > Reverted in r238007. Could you reorganize MIR to dissolve
> dependencies between LLVMCodeGen and LLVMMIR?
> > > >
> > > >
> > > > /llvm/trunk/lib/CodeGen/MIR/MIRPrintingPass.cpp:18 This depends on
> LLVMCodeGen.
> > > > /llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp:153 This depends on
> LLVMMIR.
> > > > /llvm/trunk/include/llvm/CodeGen/Passes.h:379 I suggest it might be
> logically in include/llvm/CodeGen/MIR.
> > > >
> > > > USERS
> > > >   arphaman (Author)
> > > >
> > > > http://reviews.llvm.org/rL237954
> > > >
> > > > EMAIL PREFERENCES
> > > >   http://reviews.llvm.org/settings/panel/emailpreferences/
> > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > llvm-commits mailing list
> > > > llvm-commits at cs.uiuc.edu
> > > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >
> > >
> > > <mir-move.patch>
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150527/25f1ff3b/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mirSerialization.patch
Type: application/octet-stream
Size: 33928 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150527/25f1ff3b/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mirDependencyReorganization.patch
Type: application/octet-stream
Size: 20174 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150527/25f1ff3b/attachment-0001.obj>