<div dir="ltr">Thanks, I committed this in r238341.<div><br></div><div>Alex.</div></div><div class="gmail_extra"><br><div class="gmail_quote">2015-05-27 10:20 GMT-07:00 Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thanks for pushing forward with the lib/MachineIR work!<br>
<br>
This LGTM.<br>
<div><div class="h5"><br>
> On 2015 May 27, at 10:03, Alex L <<a href="mailto:arphaman@gmail.com">arphaman@gmail.com</a>> wrote:<br>
><br>
> I've decided that the approach where MIR printing is in lib/CodeGen and the parsing is in lib/CodeGen/MIRParser will work<br>
> well for now - the circular dependency will no longer exist as CodeGen won't depend on MIRParser.<br>
> The MIR serialization printing and parsing can also be moved when the rest of the Machine IR is moved out of CodeGen,<br>
> so I think it's best to commit it before that so that I can work on both in parallel.<br>
><br>
> I've attached an updated patch (mirSerialization.patch) that has the reverted commit with the MIRParser library reorganization.<br>
> I've also attached a second patch that shows the reorganization diff between the reverted commit and the updated patch.<br>
><br>
> Cheers,<br>
> Alex<br>
><br>
><br>
><br>
> 2015-05-22 15:31 GMT-07:00 Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>>:<br>
><br>
> > On 2015-May-22, at 15:15, Alex L <<a href="mailto:arphaman@gmail.com">arphaman@gmail.com</a>> wrote:<br>
> ><br>
> ><br>
> ><br>
> > 2015-05-22 15:09 GMT-07:00 Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>>:<br>
> ><br>
> > > On 2015-May-22, at 14:11, Alex L <<a href="mailto:arphaman@gmail.com">arphaman@gmail.com</a>> wrote:<br>
> > ><br>
> > ><br>
> > ><br>
> > > 2015-05-22 11:00 GMT-07:00 Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>>:<br>
> > > Why not move the machine IR state into lib/CodeGen/MIR?  Then it<br>
> > > won't depend on lib/CodeGen.<br>
> > ><br>
> > > While moving the cpp files for MachineFunction, MachineBasicBlock, etc. to MIR<br>
> > > would be easy, moving the headers will require changes to all the targets. At the same time the headers<br>
> > > can stay under CodeGen just like the header file for AsmPrinter, but I would prefer either to move<br>
> > > everything or move nothing at all. I attached a patch below that moves the necessary files from CodeGen<br>
> > > to a new library called MIR, but I don't think this kind of change will be accepted.<br>
> ><br>
> > Why not?  This patch makes sense to me.  I think it's a good<br>
> > cleanup *anyway*.<br>
> ><br>
> > (Deserves its own thread though, since as you indicate it's a<br>
> > fairly invasive change.)<br>
> ><br>
> > 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<br>
> > what happens.<br>
><br>
> In the short term, I'm fine with:<br>
><br>
>   - lib/CodeGen: machine IR + the rest of CodeGen<br>
>   - lib/CodeGen/MIRParser: machine IR parser<br>
><br>
> as long as you keep pushing for better organization in parallel.<br>
> I don't imagine you'll get any resistance, though.<br>
><br>
> > > I think the best approach here would be to move everything from CodeGen/MIR to CodeGen so that<br>
> > > the serialization will be inside of a CodeGen library. The approach that I outlined in my previous email where<br>
> > > only the MIRParser is a separate library should also work.<br>
> ><br>
> > Having a separate library for the parser sounds like a good idea<br>
> > regardless.  This makes sense to me:<br>
> ><br>
> >   - lib/MIR: machine IR<br>
> >   - lib/MIR/MIRParser: machine IR parser<br>
> ><br>
> > ><br>
> > > Cheers,<br>
> > > Alex.<br>
> > ><br>
> > ><br>
> > > > On 2015 May 22, at 09:38, Alex L <<a href="mailto:arphaman@gmail.com">arphaman@gmail.com</a>> wrote:<br>
> > > ><br>
> > > > Hi,<br>
> > > ><br>
> > > > I'm not sure if it would possible to break this circular dependency.<br>
> > > ><br>
> > > > LLVMMIR has to depend on LLVMCodeGen as it needs to access all the machine IR state.<br>
> > > > But at the same time LLVMCodeGen has to connect to LLVMMIR to create a printing pass and<br>
> > > > then later to parse the machine functions, as llc does all the pass setup with LLVMCodeGen.<br>
> > > ><br>
> > > > If this dependency is a problem maybe LLVMMIR should be a part of LLVMCodeGen?<br>
> > > ><br>
> > > > Alex.<br>
> > > ><br>
> > > > 2015-05-22 7:48 GMT-07:00 NAKAMURA Takumi <<a href="mailto:geek4civic@gmail.com">geek4civic@gmail.com</a>>:<br>
> > > > Reverted in r238007. Could you reorganize MIR to dissolve dependencies between LLVMCodeGen and LLVMMIR?<br>
> > > ><br>
> > > ><br>
> > > > /llvm/trunk/lib/CodeGen/MIR/MIRPrintingPass.cpp:18 This depends on LLVMCodeGen.<br>
> > > > /llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp:153 This depends on LLVMMIR.<br>
> > > > /llvm/trunk/include/llvm/CodeGen/Passes.h:379 I suggest it might be logically in include/llvm/CodeGen/MIR.<br>
> > > ><br>
> > > > USERS<br>
> > > >   arphaman (Author)<br>
> > > ><br>
> > > > <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_rL237954&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=4yswys4FBSDCzKZx9VqGptj-a8foJ6UjnmLMj6kkcS0&s=8uRu8kmAkPujr9eFbgLIF1mdffD_9nI1RfMV7Ulotes&e=" target="_blank">http://reviews.llvm.org/rL237954</a><br>
> > > ><br>
> > > > EMAIL PREFERENCES<br>
> > > >   <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=4yswys4FBSDCzKZx9VqGptj-a8foJ6UjnmLMj6kkcS0&s=t2dW7b-dfbnTYoi_SMB1fBkFlUSIXUsz5oJSz2vVZ6U&e=" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
> > > ><br>
> > > ><br>
> > > ><br>
> > > > _______________________________________________<br>
> > > > llvm-commits mailing list<br>
> > > > <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> > > > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
> > ><br>
> > ><br>
> > > <mir-move.patch><br>
> ><br>
> ><br>
><br>
><br>
</div></div>> <mirSerialization.patch><mirDependencyReorganization.patch><br>
<br>
</blockquote></div><br></div>