|Show First 20 Lines • Show All 94 Lines • ▼ Show 20 Lines||add_llvm_library(LLVMCodeGen|
Looks like this patch could be split:
qcolombet: Looks like this patch could be split: - One patch for RegisterUsageInfo. - One patch for…
@qcolombet Could you please explain why is that required?
vivekvpandya: @qcolombet Could you please explain why is that required?
It is a good practice to decoupled software component. Having separate patches helps to make sure we indeed have correctly separated the components.
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: It is a good practice to decoupled software component. Having separate patches helps to make…
@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) .
vivekvpandya: @mehdi_amini I understand what you explain above but here I think RegisterUsageInfo is not…
Splitting in two is what I had in mind: the analysis part on one side, the transformation part on another.
mehdi_amini: Splitting in two is what I had in mind: the analysis part on one side, the transformation part…
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.
vivekvpandya: Just to make sure RegisterUsageInfo.cpp and RegUsageInfoCollector.cpp both are part of analysis…
|Show All 36 Lines|