llvm dev - May 2015 - [LLVMdev] RFC: Separate machine IR from lib/CodeGen into lib/MIR

2015-05-27 10:59 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at
apple.com>:
> > On 2015 May 27, at 10:24, Chandler Carruth <chandlerc at
google.com> wrote:
> >
> >> On Wed, May 27, 2015 at 8:15 AM Chris Lattner <clattner at
apple.com>
> wrote:
> >>> On May 26, 2015, at 11:20 PM, Quentin Colombet <qcolombet
at apple.com>
> wrote:
> >>>
> >>> +1.
> >>>
> >>> Could those two be subdirectories of one
“Machine-Related-Stuff”
> directory?
> >>> E.g.,
> >>> MachineStuff/IR
> >>> MachineStuff/CodeGen
> >>>
> >>> Where MachineStuff is something meaningful :).
> >>>
> >>> That way, they keep a logic bound, more formal than the naming
> convention.
> >>
> >> Something like?
> >>
> >> lib/Machine/IR
> >> lib/Machine/Passes
> >>
> >> Unless there will be many subdirectories, it seems slightly better
to
> flatten out the layer.
> >
> > I strongly prefer breaking it out into subdirectories. There are a
bunch
> of reasons:
> >
> > 1) It will grow.
> > 2) Without it, we cannot have separate libraries, which will lose some
> options for shrinking the size of libraries.
> > 3) Without separate libraries we can't as easily enforce the
layering
> separations between these things. For example, making this split will be
> *extremely* hard currently because there is a lot of inappropriate
> dependencies that will block splitting things out.
> >
> > However, "IR" and "Passes" cover only two of the
things in CodeGen.
> There is also the implementation of a lot of common infrastructure used by
> targets' code generators.
> >
> > My initial suggestion would be to just sink CodeGen into
> Machine/CodeGen, add the .../IR and .../Passes directories, and then start
> extracting things from CodeGen into the two more narrow directories. I
> think there is likely some stuff that should continue to live in a
"code
> generator" infrastructure directory as it is neither part of the
machine
> IR, nor is it part of any particular pass.
> >
> > My suggested layering would be:
> >
> > Passes depend on IR, CodeGen depends on both Passes and IR. The idea
is
> that anything passes require should be embedded into the IR.
> >
>
> (Oops, missed this until after I sent my own response.
>
> One thing I'd add to this from my email is that I think
> lib/Machine/IR is likely to get confused with lib/IR for the same
> reasons that lib/CodeGen is confusing between LLVM and Clang.  IMO
> lib/Machine/MIR is "safer".)
>
> > However, this won't currently work. There are things that seem to
be
> parallel but independent of the machine IR and are used by any machine
> passes. There are also things that clearly use the machine passes.
> Currently, I'm not sure how to cleanly divide this library up without
> really significant refactoring of every part of the code generator.
> >
> > While I would like to see this happen, is it really a good idea to put
> this in the critical path of getting MIR serialized and deserialized?
>
> Not if it's as hard as you're saying.  My impression was that Alex
> was able to move the IR stuff he needed into a separately library
> pretty trivially (based on the P.O.C. patch he posted), but if it's
> not trivial he should just move on.
>
While it's true that there are things that prevent a straightforward
library division, I don't think that they will pose such a big problem.
I have a brief list of things that have to be done before I can move the
Machine IR stuff from CodeGen to some other library:
- Move the SplitCriticalEdge method from MachineBasicBlock (
http://reviews.llvm.org/D10064).
- Move the UnpackMachineBundles and FinalizeMachineBundles passes from
MachineInstrBundle.cpp. (http://reviews.llvm.org/D10070 + 1 upcoming patch).
- Refactor SlotIndexes.h: keep the SlotIndexes pass and move the rest to
SlotIndex.h. Introduce a new container class in SlotIndex.h that will
extract the map
  between machine instructions and slot indexes from the SlotIndexes pass
and remove the dependency on the pass from MachineBasicBlock's print method.
- WinEHFuncInfo - Either move parseEHInfo from WinEHPrepare.cpp to new file
WinEHFuncInfo.cpp or move the declaration of parseEHInfo from
WinEHFuncInfo.h
  to a new header WinEHPrepare.h
- Get rid of several unused #includes in MachineIR cpp files that include
passes.

After that it should be possible to move the Machine IR files out of
CodeGen without them actually including any CodeGen headers.
And all the Machine IR passes like MachineFunctionPass,
MachineFunctionAnalysis, etc. will remain in CodeGen.

Alex.


>
> >> Also, if we’re getting crazy here, CodeGen in clang should be
renamed
> to IRGen, AsmPrinter should be renamed to MCGen, and SelectionDAG should be
> replaced ;-)
> >
> > I'm happy to actually do the CodeGen -> IRGen rename. I
actually built
> the change but didn't submit it because of the concerns of some
out-of-tree
> users of Clang. I still have all the perl scripts and such I used sitting
> around.
>
> I'm a really big fan of this.  If you supply the perl scripts
> somehow (attached to a PR that you reference in the commit?) then
> out-of-tree users shouldn't have a problem.  Unless I'm missing
> something?
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150527/3780fc14/attachment.html>

> On 2015 May 27, at 14:01, Alex L <arphaman at gmail.com> wrote:
> 
> 
> 
> 2015-05-27 10:59 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at
apple.com>:
> > On 2015 May 27, at 10:24, Chandler Carruth <chandlerc at
google.com> wrote:
> >
> >> On Wed, May 27, 2015 at 8:15 AM Chris Lattner <clattner at
apple.com> wrote:
> >>> On May 26, 2015, at 11:20 PM, Quentin Colombet <qcolombet
at apple.com> wrote:
> >>>
> >>> +1.
> >>>
> >>> Could those two be subdirectories of one
“Machine-Related-Stuff” directory?
> >>> E.g.,
> >>> MachineStuff/IR
> >>> MachineStuff/CodeGen
> >>>
> >>> Where MachineStuff is something meaningful :).
> >>>
> >>> That way, they keep a logic bound, more formal than the naming
convention.
> >>
> >> Something like?
> >>
> >> lib/Machine/IR
> >> lib/Machine/Passes
> >>
> >> Unless there will be many subdirectories, it seems slightly better
to flatten out the layer.
> >
> > I strongly prefer breaking it out into subdirectories. There are a
bunch of reasons:
> >
> > 1) It will grow.
> > 2) Without it, we cannot have separate libraries, which will lose some
options for shrinking the size of libraries.
> > 3) Without separate libraries we can't as easily enforce the
layering separations between these things. For example, making this split will
be *extremely* hard currently because there is a lot of inappropriate
dependencies that will block splitting things out.
> >
> > However, "IR" and "Passes" cover only two of the
things in CodeGen. There is also the implementation of a lot of common
infrastructure used by targets' code generators.
> >
> > My initial suggestion would be to just sink CodeGen into
Machine/CodeGen, add the .../IR and .../Passes directories, and then start
extracting things from CodeGen into the two more narrow directories. I think
there is likely some stuff that should continue to live in a "code
generator" infrastructure directory as it is neither part of the machine
IR, nor is it part of any particular pass.
> >
> > My suggested layering would be:
> >
> > Passes depend on IR, CodeGen depends on both Passes and IR. The idea
is that anything passes require should be embedded into the IR.
> >
> 
> (Oops, missed this until after I sent my own response.
> 
> One thing I'd add to this from my email is that I think
> lib/Machine/IR is likely to get confused with lib/IR for the same
> reasons that lib/CodeGen is confusing between LLVM and Clang.  IMO
> lib/Machine/MIR is "safer".)
> 
> > However, this won't currently work. There are things that seem to
be parallel but independent of the machine IR and are used by any machine
passes. There are also things that clearly use the machine passes. Currently,
I'm not sure how to cleanly divide this library up without really
significant refactoring of every part of the code generator.
> >
> > While I would like to see this happen, is it really a good idea to put
this in the critical path of getting MIR serialized and deserialized?
> 
> Not if it's as hard as you're saying.  My impression was that Alex
> was able to move the IR stuff he needed into a separately library
> pretty trivially (based on the P.O.C. patch he posted), but if it's
> not trivial he should just move on.
> 
> While it's true that there are things that prevent a straightforward
library division, I don't think that they will pose such a big problem.
> I have a brief list of things that have to be done before I can move the
Machine IR stuff from CodeGen to some other library:
> - Move the SplitCriticalEdge method from MachineBasicBlock
(http://reviews.llvm.org/D10064).
> - Move the UnpackMachineBundles and FinalizeMachineBundles passes from
MachineInstrBundle.cpp. (http://reviews.llvm.org/D10070 + 1 upcoming patch).
> - Refactor SlotIndexes.h: keep the SlotIndexes pass and move the rest to
SlotIndex.h. Introduce a new container class in SlotIndex.h that will extract
the map
>   between machine instructions and slot indexes from the SlotIndexes pass
and remove the dependency on the pass from MachineBasicBlock's print method.
> - WinEHFuncInfo - Either move parseEHInfo from WinEHPrepare.cpp to new file
WinEHFuncInfo.cpp or move the declaration of parseEHInfo from WinEHFuncInfo.h
>   to a new header WinEHPrepare.h
> - Get rid of several unused #includes in MachineIR cpp files that include
passes.
> 
> After that it should be possible to move the Machine IR files out of
CodeGen without them actually including any CodeGen headers.
> And all the Machine IR passes like MachineFunctionPass,
MachineFunctionAnalysis, etc. will remain in CodeGen.
> 
> Alex.
> 
Something Chandler pointed out on IRC is that #include dependencies
aren't enough -- we also need to root out any symbol dependencies.
Have you checked for symbols (mostly, functions) that are declared
in a header you're planning to move, but defined in a source file
you're *not* planning to move?

(Apparently there have been failed attempts in the past to bring
sanity here that I wasn't aware of...)
>  
> 
> >> Also, if we’re getting crazy here, CodeGen in clang should be
renamed to IRGen, AsmPrinter should be renamed to MCGen, and SelectionDAG should
be replaced ;-)
> >
> > I'm happy to actually do the CodeGen -> IRGen rename. I
actually built the change but didn't submit it because of the concerns of
some out-of-tree users of Clang. I still have all the perl scripts and such I
used sitting around.
> 
> I'm a really big fan of this.  If you supply the perl scripts
> somehow (attached to a PR that you reference in the commit?) then
> out-of-tree users shouldn't have a problem.  Unless I'm missing
> something?
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

I didn't specifically check for them, but the WinEHFuncInfo header file
problem that I described above
is an example of a symbol dependency that can be resolved in a fairly
straight forward manner.

The remaining files seem pretty sane, but there might be some symbol
dependencies that I haven't
caught yet.

Alex.

2015-05-27 14:18 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at
apple.com>:
>
> > On 2015 May 27, at 14:01, Alex L <arphaman at gmail.com> wrote:
> >
> >
> >
> > 2015-05-27 10:59 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at
apple.com
> >:
> > > On 2015 May 27, at 10:24, Chandler Carruth <chandlerc at
google.com>
> wrote:
> > >
> > >> On Wed, May 27, 2015 at 8:15 AM Chris Lattner <clattner at
apple.com>
> wrote:
> > >>> On May 26, 2015, at 11:20 PM, Quentin Colombet
<qcolombet at apple.com>
> wrote:
> > >>>
> > >>> +1.
> > >>>
> > >>> Could those two be subdirectories of one
“Machine-Related-Stuff”
> directory?
> > >>> E.g.,
> > >>> MachineStuff/IR
> > >>> MachineStuff/CodeGen
> > >>>
> > >>> Where MachineStuff is something meaningful :).
> > >>>
> > >>> That way, they keep a logic bound, more formal than the
naming
> convention.
> > >>
> > >> Something like?
> > >>
> > >> lib/Machine/IR
> > >> lib/Machine/Passes
> > >>
> > >> Unless there will be many subdirectories, it seems slightly
better to
> flatten out the layer.
> > >
> > > I strongly prefer breaking it out into subdirectories. There are
a
> bunch of reasons:
> > >
> > > 1) It will grow.
> > > 2) Without it, we cannot have separate libraries, which will lose
some
> options for shrinking the size of libraries.
> > > 3) Without separate libraries we can't as easily enforce the
layering
> separations between these things. For example, making this split will be
> *extremely* hard currently because there is a lot of inappropriate
> dependencies that will block splitting things out.
> > >
> > > However, "IR" and "Passes" cover only two of
the things in CodeGen.
> There is also the implementation of a lot of common infrastructure used by
> targets' code generators.
> > >
> > > My initial suggestion would be to just sink CodeGen into
> Machine/CodeGen, add the .../IR and .../Passes directories, and then start
> extracting things from CodeGen into the two more narrow directories. I
> think there is likely some stuff that should continue to live in a
"code
> generator" infrastructure directory as it is neither part of the
machine
> IR, nor is it part of any particular pass.
> > >
> > > My suggested layering would be:
> > >
> > > Passes depend on IR, CodeGen depends on both Passes and IR. The
idea
> is that anything passes require should be embedded into the IR.
> > >
> >
> > (Oops, missed this until after I sent my own response.
> >
> > One thing I'd add to this from my email is that I think
> > lib/Machine/IR is likely to get confused with lib/IR for the same
> > reasons that lib/CodeGen is confusing between LLVM and Clang.  IMO
> > lib/Machine/MIR is "safer".)
> >
> > > However, this won't currently work. There are things that
seem to be
> parallel but independent of the machine IR and are used by any machine
> passes. There are also things that clearly use the machine passes.
> Currently, I'm not sure how to cleanly divide this library up without
> really significant refactoring of every part of the code generator.
> > >
> > > While I would like to see this happen, is it really a good idea
to put
> this in the critical path of getting MIR serialized and deserialized?
> >
> > Not if it's as hard as you're saying.  My impression was that
Alex
> > was able to move the IR stuff he needed into a separately library
> > pretty trivially (based on the P.O.C. patch he posted), but if
it's
> > not trivial he should just move on.
> >
> > While it's true that there are things that prevent a
straightforward
> library division, I don't think that they will pose such a big problem.
> > I have a brief list of things that have to be done before I can move
the
> Machine IR stuff from CodeGen to some other library:
> > - Move the SplitCriticalEdge method from MachineBasicBlock (
> http://reviews.llvm.org/D10064).
> > - Move the UnpackMachineBundles and FinalizeMachineBundles passes from
> MachineInstrBundle.cpp. (http://reviews.llvm.org/D10070 + 1 upcoming
> patch).
> > - Refactor SlotIndexes.h: keep the SlotIndexes pass and move the rest
to
> SlotIndex.h. Introduce a new container class in SlotIndex.h that will
> extract the map
> >   between machine instructions and slot indexes from the SlotIndexes
> pass and remove the dependency on the pass from MachineBasicBlock's
print
> method.
> > - WinEHFuncInfo - Either move parseEHInfo from WinEHPrepare.cpp to new
> file WinEHFuncInfo.cpp or move the declaration of parseEHInfo from
> WinEHFuncInfo.h
> >   to a new header WinEHPrepare.h
> > - Get rid of several unused #includes in MachineIR cpp files that
> include passes.
> >
> > After that it should be possible to move the Machine IR files out of
> CodeGen without them actually including any CodeGen headers.
> > And all the Machine IR passes like MachineFunctionPass,
> MachineFunctionAnalysis, etc. will remain in CodeGen.
> >
> > Alex.
> >
>
> Something Chandler pointed out on IRC is that #include dependencies
> aren't enough -- we also need to root out any symbol dependencies.
> Have you checked for symbols (mostly, functions) that are declared
> in a header you're planning to move, but defined in a source file
> you're *not* planning to move?
>
> (Apparently there have been failed attempts in the past to bring
> sanity here that I wasn't aware of...)
>
> >
> >
> > >> Also, if we’re getting crazy here, CodeGen in clang should be
renamed
> to IRGen, AsmPrinter should be renamed to MCGen, and SelectionDAG should be
> replaced ;-)
> > >
> > > I'm happy to actually do the CodeGen -> IRGen rename. I
actually built
> the change but didn't submit it because of the concerns of some
out-of-tree
> users of Clang. I still have all the perl scripts and such I used sitting
> around.
> >
> > I'm a really big fan of this.  If you supply the perl scripts
> > somehow (attached to a PR that you reference in the commit?) then
> > out-of-tree users shouldn't have a problem.  Unless I'm
missing
> > something?
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150527/4429bb49/attachment.html>

> On May 27, 2015, at 2:18 PM, Duncan P. N. Exon Smith <dexonsmith at
apple.com> wrote:
> 
> 
>> On 2015 May 27, at 14:01, Alex L <arphaman at gmail.com> wrote:
>> 
>> 
>> 
>> 2015-05-27 10:59 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at
apple.com>:
>>> On 2015 May 27, at 10:24, Chandler Carruth <chandlerc at
google.com> wrote:
>>> 
>>>> On Wed, May 27, 2015 at 8:15 AM Chris Lattner <clattner at
apple.com> wrote:
>>>>> On May 26, 2015, at 11:20 PM, Quentin Colombet
<qcolombet at apple.com> wrote:
>>>>> 
>>>>> +1.
>>>>> 
>>>>> Could those two be subdirectories of one
“Machine-Related-Stuff” directory?
>>>>> E.g.,
>>>>> MachineStuff/IR
>>>>> MachineStuff/CodeGen
>>>>> 
>>>>> Where MachineStuff is something meaningful :).
>>>>> 
>>>>> That way, they keep a logic bound, more formal than the
naming convention.
>>>> 
>>>> Something like?
>>>> 
>>>> lib/Machine/IR
>>>> lib/Machine/Passes
>>>> 
>>>> Unless there will be many subdirectories, it seems slightly
better to flatten out the layer.
>>> 
>>> I strongly prefer breaking it out into subdirectories. There are a
bunch of reasons:
>>> 
>>> 1) It will grow.
>>> 2) Without it, we cannot have separate libraries, which will lose
some options for shrinking the size of libraries.
>>> 3) Without separate libraries we can't as easily enforce the
layering separations between these things. For example, making this split will
be *extremely* hard currently because there is a lot of inappropriate
dependencies that will block splitting things out.
>>> 
>>> However, "IR" and "Passes" cover only two of
the things in CodeGen. There is also the implementation of a lot of common
infrastructure used by targets' code generators.
>>> 
>>> My initial suggestion would be to just sink CodeGen into
Machine/CodeGen, add the .../IR and .../Passes directories, and then start
extracting things from CodeGen into the two more narrow directories. I think
there is likely some stuff that should continue to live in a "code
generator" infrastructure directory as it is neither part of the machine
IR, nor is it part of any particular pass.
>>> 
>>> My suggested layering would be:
>>> 
>>> Passes depend on IR, CodeGen depends on both Passes and IR. The
idea is that anything passes require should be embedded into the IR.
>>> 
>> 
>> (Oops, missed this until after I sent my own response.
>> 
>> One thing I'd add to this from my email is that I think
>> lib/Machine/IR is likely to get confused with lib/IR for the same
>> reasons that lib/CodeGen is confusing between LLVM and Clang.  IMO
>> lib/Machine/MIR is "safer".)
>> 
>>> However, this won't currently work. There are things that seem
to be parallel but independent of the machine IR and are used by any machine
passes. There are also things that clearly use the machine passes. Currently,
I'm not sure how to cleanly divide this library up without really
significant refactoring of every part of the code generator.
>>> 
>>> While I would like to see this happen, is it really a good idea to
put this in the critical path of getting MIR serialized and deserialized?
>> 
>> Not if it's as hard as you're saying.  My impression was that
Alex
>> was able to move the IR stuff he needed into a separately library
>> pretty trivially (based on the P.O.C. patch he posted), but if it's
>> not trivial he should just move on.
>> 
>> While it's true that there are things that prevent a
straightforward library division, I don't think that they will pose such a
big problem.
>> I have a brief list of things that have to be done before I can move
the Machine IR stuff from CodeGen to some other library:
>> - Move the SplitCriticalEdge method from MachineBasicBlock
(http://reviews.llvm.org/D10064).
>> - Move the UnpackMachineBundles and FinalizeMachineBundles passes from
MachineInstrBundle.cpp. (http://reviews.llvm.org/D10070 + 1 upcoming patch).
>> - Refactor SlotIndexes.h: keep the SlotIndexes pass and move the rest
to SlotIndex.h. Introduce a new container class in SlotIndex.h that will extract
the map
>>  between machine instructions and slot indexes from the SlotIndexes
pass and remove the dependency on the pass from MachineBasicBlock's print
method.
>> - WinEHFuncInfo - Either move parseEHInfo from WinEHPrepare.cpp to new
file WinEHFuncInfo.cpp or move the declaration of parseEHInfo from
WinEHFuncInfo.h
>>  to a new header WinEHPrepare.h
>> - Get rid of several unused #includes in MachineIR cpp files that
include passes.
>> 
>> After that it should be possible to move the Machine IR files out of
CodeGen without them actually including any CodeGen headers.
>> And all the Machine IR passes like MachineFunctionPass,
MachineFunctionAnalysis, etc. will remain in CodeGen.
>> 
>> Alex.
>> 
> 
> Something Chandler pointed out on IRC is that #include dependencies
> aren't enough -- we also need to root out any symbol dependencies.
> Have you checked for symbols (mostly, functions) that are declared
> in a header you're planning to move, but defined in a source file
> you're *not* planning to move?
> 
> (Apparently there have been failed attempts in the past to bring
> sanity here that I wasn't aware of…)
The solution to this is to have a tool which links only the parts we need.  So
for example, write a tool which only verifies machine IR.  We parse it and
verify it, and exit.  That only needs the MIR and parser and will root out any
dependencies on IR/CodeGen.

+1 for everything else BTW, not that we needed another +1.

Cheers,
Pete
> 
>> 
>> 
>>>> Also, if we’re getting crazy here, CodeGen in clang should be
renamed to IRGen, AsmPrinter should be renamed to MCGen, and SelectionDAG should
be replaced ;-)
>>> 
>>> I'm happy to actually do the CodeGen -> IRGen rename. I
actually built the change but didn't submit it because of the concerns of
some out-of-tree users of Clang. I still have all the perl scripts and such I
used sitting around.
>> 
>> I'm a really big fan of this.  If you supply the perl scripts
>> somehow (attached to a PR that you reference in the commit?) then
>> out-of-tree users shouldn't have a problem.  Unless I'm missing
>> something?
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev