<div dir="ltr"><br><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></blockquote><div><br></div><div>I would prefer to go with the short term approach with my patch where the MIRParser is a separate lib</div><div>under CodeGen for now.</div><div><br></div><div>I will submit a proposal for CodeGen to MIR reorganization, but it seems like it will take more than a simple</div><div>rename to reorganize things correctly, as some MIR files need headers certain CodeGen passes and a lot </div><div>of them even include 'llvm/CodeGen/Passes.h'. The simple rename will just bring us another circular dependency,</div><div>and thus it would probably take a couple of commits to reorganize things in CodeGen and then to do a rename.</div><div>This would take some time, so for now I would like to commit the MIR printing code under CodeGen and the </div><div>MIR parsing code under CodeGen/MIRParser and work in parallel on MIR serialization and MIR reorganization.</div><div><br></div><div>Cheers,</div><div>Alex</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<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=WNhV_KeGJ9HoIEJSr9stGmm_u-nFIhr2COK5TyvTlDw&s=TwC2vDS_hGvR2-0cTBBMNHp1pRf4qqUpDFynlM5ezJA&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=WNhV_KeGJ9HoIEJSr9stGmm_u-nFIhr2COK5TyvTlDw&s=HMmxXUT6m6GJOZ-x6bO35WNHzWlDhtu4Nwe7oudt36A&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></div>