> So yeah, I agree this is right after all, sorry. Let's delete the > comment starting at "There are two problems here:" at the same time.
Ok. > mips_regno_to_class should then map $sp to the new class, since it's now > the smallest containing class. (We really should set that up automatically > one day...) Indeed, it is easy to overlook it. > How about M16_SP_REGS, to match M16_T_REGS? > > Also, the BASE_REG_CLASS/ADDR_REG_CLASS distinction isn't all that > obvious from the names. ADDR_REG_CLASS is only needed for the "d" > constraint so maybe we could just use TARGET_MIPS16 ? M16_REGS : GR_REGS > directly for now. Fair enough. I'll remove ADDR_REG_CLASS. > I can imagine including all M16_REGS makes sense, but it seems odd to > drop the 2 temporaries. Does 0x0303fffc have the same problem? It's a midway between 0x0003fffc and 0x0300fffc. I think 0x0303fffc will be a good trade-off. The difference between 0x0303fffc and others is about ~600 bytes in CSiBE which is less than 0.01% of code size change. > Hmm, marking them fixed was supposed to be a temporary reload-only thing, > until the move to LRA. It should never be worse to spill to these GPRs > over spilling to the stack, if the value isn't live across a call. I would say this also affects IRA/LRA integration. I found that it is more profitable to hide registers (MIPS16 only) in IRA to encourage spilling to memory. Otherwise $8-$15 would be treated like any other registers and LRA would inserts reloads to move in/out values of these registers. My assumption is that if we could hide some of the registers in IRA but enable them in LRA then all registers in SPILL_REGS would be available keeping reasonable code size. Another way would be to increase the cost of moving values between M16_REGS and GR_REGS but it was already mentioned, and is true that there should be no difference of costs and it feels like a hack to make things work. > Did you see the failures even after your mips_regno_mode_ok_for_base_p > change? LRA should know how to reload a "W" address. Yes but I realize there is more. It fails because $sp is now included in BASE_REG_CLASS and "W" is based on it. However, I suppose that it would be too eager to say it is wrong and likely there is something missing in LRA if we want to keep all alternatives. Currently there is no check if a reloaded operand has a valid address, use of $sp in lbu/lhu cases. Even if we added extra checks we are less likely to benefit as we need to reload the base into register. > Even that might be too loose, since invalid scales will need to be reloaded > as a multiplication or shift, and there's no guarantee that the target > can do that without clobbering the flags. So maybe we should do something > like the patch below. > > Alternatively we could stick to the decompose_mem_address-based check > above and teach LRA to keep invalid addresses for 'X'. The problem then > is that we might ICE while printing the operand. Tightening validity for 'X' appears to be reasonable. Will you commit this patch? Regards, Robert
