[PATCH] D20769: [IPRA] Interprocedural Register Allocation

[Vivek Pandya via llvm-commits]

vivekvpandya added inline comments.

================
Comment at: lib/CodeGen/CMakeLists.txt:103
@@ -102,2 +102,3 @@
   RegisterScavenging.cpp
   RenameIndependentSubregs.cpp
+  RegisterUsageInfo.cpp
----------------
mehdi_amini wrote:
> vivekvpandya wrote:
> > mehdi_amini wrote:
> > > It is a good practice to decoupled software component. Having separate patches helps to make sure we indeed have correctly separated the components.
> > > It also forces to make sure the component are individually testable, and make sure we actually test them.
> > > 
> > > So I agree with Quentin on the principle, and I think it is also a good exercise for you to split the patch an submit the analysis alone and tested. Have a look at "CostModelAnalysis::print()" in  lib/Analysis/CostModel.cpp and see how it is tested in  test/Analysis/CostModel/X86/cast.ll
> > > 
> > > 
> > @mehdi_amini I understand what you explain above but here I think RegisterUsageInfo is not tastable alone because it just holds RegMasks, RegisterInfoCollector is a trigger to IP regalloc and both of them can be tested together. Also to test X86RegUsageInfoPropagate it requires both of the above mentioned passes. But we can separate patches for RegisterUsageInfo + InfoCollector and X86RegUsageInfoPropagate ( condition that first patch is required to test second one) .
> > Is there any better plan in your mind?
> Splitting in two is what I had in mind: the analysis part on one side, the transformation part on another.
Just to make sure RegisterUsageInfo.cpp and RegUsageInfoCollector.cpp both are part of analysis so there is not need of separate patch for them but I will separate changes related to X86RegUsageInfoPropagate.cpp in other patch.


http://reviews.llvm.org/D20769