<div dir="ltr">I've decided that the approach where MIR printing is in lib/CodeGen and the parsing is in lib/CodeGen/MIRParser will work <div>well for now - the circular dependency will no longer exist as CodeGen won't depend on MIRParser.<div>The MIR serialization printing and parsing can also be moved when the rest of the Machine IR is moved out of CodeGen, </div><div>so I think it's best to commit it before that so that I can work on both in parallel.</div><div><br></div><div>I've attached an updated patch (mirSerialization.patch) that has the reverted commit with the MIRParser library reorganization. </div><div>I've also attached a second patch that shows the reorganization diff between the reverted commit and the updated patch.</div></div><div><br></div><div>Cheers,</div><div>Alex</div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">2015-05-22 15:31 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"><span class=""><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>
</span>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>
<div class="HOEnZb"><div class="h5"><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=fCVRe71dWXLeYGCqRUPfKD7r_Vzavv6l9ie5d8CW8WM&s=sCNtcvor27oeLmFBb4B9xM_uisMkk1VK2m5LZCJMkvw&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=fCVRe71dWXLeYGCqRUPfKD7r_Vzavv6l9ie5d8CW8WM&s=4iD5S5GmFipm4OQ1FuMf1zbh56rgSy_gBd9m0vXfMGk&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>
</div></div></blockquote></div><br></div>