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

[Alex L]

Thanks, I committed this in r238341.

Alex.

2015-05-27 10:20 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:

> Thanks for pushing forward with the lib/MachineIR work!
>
> This LGTM.
>
> > On 2015 May 27, at 10:03, Alex L <arphaman at gmail.com> wrote:
> >
> > 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>
> > >
> > >
> >
> >
> > <mirSerialization.patch><mirDependencyReorganization.patch>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150527/4af4bb9a/attachment.html>