| Message ID | 87bohi48tv.fsf@talisman.home |
|---|---|
| State | New |
| Headers | show |
On 09/07/2012 02:59 AM, Richard Sandiford wrote: > ira.c:setup_pressure_classes treats "leaf" classes as pressure classes > even if moves between them are more expensive than moves to or from memory: > > if (ira_class_hard_regs_num[cl] != 1 > /* A register class without subclasses may contain a few > hard registers and movement between them is costly > (e.g. SPARC FPCC registers). We still should consider it > as a candidate for a pressure class. */ > && alloc_reg_class_subclasses[cl][0] != LIM_REG_CLASSES) > { > > MIPS accumulators are another case where this inclusion is important. > > Everything works as expected with current sources, where: > > MD_REGS = MD0_REG + MD1_REG > ACC_REGS = MD_REGS + DSP_ACC_REGS > > So the three leaf classes are the singleton MD0_REG and MD1_REG (available > on all MIPS targets) and DSP_ACC_REGS (available only when using the DSP ASE). > All three are treated as pressure classes where appropriate. > > However, MD0 and MD1 are no longer independently allocatable. Splitting > what is effectively one register into two classes creates some confusion, > so I'd like to get rid of them. We're then left with just: > > ACC_REGS = MD_REGS + DSP_ACC_REGS > > where MD_REGS and DSP_ACC_REGS are both leaf classes. Without the DSP ASE, > that reduces to: > > ACC_REGS = MD_REGS > > The problem is that alloc_reg_class_subclasses (unlike reg_class_subclasses) > includes higher-numbered classes too. So when ACC_REGS = MD_REGS, > alloc_reg_class_subclasses lists ACC_REGS as a subset of MD_REGS > and MD_REGS as a subset of ACC_REGS. The condition: > > alloc_reg_class_subclasses[cl][0] != LIM_REG_CLASSES > > therefore holds for both of them. This is similar to the situation > mentioned in setup_uniform_class_p: > > /* We can not use alloc_reg_class_subclasses here because move > cost hooks does not take into account that some registers are > unavailable for the subtarget. E.g. for i686, INT_SSE_REGS > is element of alloc_reg_class_subclasses for GENERAL_REGS > because SSE regs are unavailable. */ > for (i = 0; (cl2 = reg_class_subclasses[cl][i]) != LIM_REG_CLASSES; i++) > > When we have several equivalent leaf classes, I think we should continue > to treat the lowest-numbered one as a pressure class. This produces the > expected results with the MIPS patch described above. > > Tested on x86_64-linux-gnu and mipsisa64-elf. Also tested by making sure > that there were no changed in cc1 .ii files for x86_64-linux-gnu. > OK to install? > Ok. It looks reasonable although I am not sure that it will work for all situations. That is a really tricky code. It is hard to say what the final outcome will be in all situations. But in what I am sure is that the change is safe. Thanks for the patch, Richard.
Index: gcc/ira.c =================================================================== --- gcc/ira.c 2012-09-05 21:33:00.115078796 +0100 +++ gcc/ira.c 2012-09-05 22:01:05.747029786 +0100 @@ -789,7 +789,7 @@ setup_pressure_classes (void) hard registers and movement between them is costly (e.g. SPARC FPCC registers). We still should consider it as a candidate for a pressure class. */ - && alloc_reg_class_subclasses[cl][0] != LIM_REG_CLASSES) + && alloc_reg_class_subclasses[cl][0] < cl) { /* Check that the moves between any hard registers of the current class are not more expensive for a legal mode
ira.c:setup_pressure_classes treats "leaf" classes as pressure classes even if moves between them are more expensive than moves to or from memory: if (ira_class_hard_regs_num[cl] != 1 /* A register class without subclasses may contain a few hard registers and movement between them is costly (e.g. SPARC FPCC registers). We still should consider it as a candidate for a pressure class. */ && alloc_reg_class_subclasses[cl][0] != LIM_REG_CLASSES) { MIPS accumulators are another case where this inclusion is important. Everything works as expected with current sources, where: MD_REGS = MD0_REG + MD1_REG ACC_REGS = MD_REGS + DSP_ACC_REGS So the three leaf classes are the singleton MD0_REG and MD1_REG (available on all MIPS targets) and DSP_ACC_REGS (available only when using the DSP ASE). All three are treated as pressure classes where appropriate. However, MD0 and MD1 are no longer independently allocatable. Splitting what is effectively one register into two classes creates some confusion, so I'd like to get rid of them. We're then left with just: ACC_REGS = MD_REGS + DSP_ACC_REGS where MD_REGS and DSP_ACC_REGS are both leaf classes. Without the DSP ASE, that reduces to: ACC_REGS = MD_REGS The problem is that alloc_reg_class_subclasses (unlike reg_class_subclasses) includes higher-numbered classes too. So when ACC_REGS = MD_REGS, alloc_reg_class_subclasses lists ACC_REGS as a subset of MD_REGS and MD_REGS as a subset of ACC_REGS. The condition: alloc_reg_class_subclasses[cl][0] != LIM_REG_CLASSES therefore holds for both of them. This is similar to the situation mentioned in setup_uniform_class_p: /* We can not use alloc_reg_class_subclasses here because move cost hooks does not take into account that some registers are unavailable for the subtarget. E.g. for i686, INT_SSE_REGS is element of alloc_reg_class_subclasses for GENERAL_REGS because SSE regs are unavailable. */ for (i = 0; (cl2 = reg_class_subclasses[cl][i]) != LIM_REG_CLASSES; i++) When we have several equivalent leaf classes, I think we should continue to treat the lowest-numbered one as a pressure class. This produces the expected results with the MIPS patch described above. Tested on x86_64-linux-gnu and mipsisa64-elf. Also tested by making sure that there were no changed in cc1 .ii files for x86_64-linux-gnu. OK to install? gcc/ * ira.c (setup_pressure_classes): Handle synonymous classes.