Peter Barada <[EMAIL PROTECTED]> writes: > >So register 1420 is being assigned to a data register. The > >constraints for addsi3_5200 permit the following alternatives: > > mem += datareg > > addrreg = addrreg + reg|constant > > addrreg = reg|constant + addrreg > > reg += mem|reg|constant > >There is no alternative which permits adding a data register and a > >constant and putting the result in an address register. So reload > >picks the alternative "addrreg = addrreg + reg|constant", and decides > >to reload register 1420 into an address register. But that fails > >because reload can't find an address register which can accept a > >QImode value. > > Would it help to rearrange the constraints to have reg += > mem|reg|constant before the addreg += ... ?
Probably not in this case. You could try it. It is true that when two alternatives have the same cost, reload will pick the first one listed. > >Looking at the dump a little more, it's far from clear why register > >1421 is being put into an address register. I see that insn 5560 > >wants to compare a byte value which it finds there, so it doesn't seem > >like a good fit to put 1421 into an address register. > > > > > >I don't know where else register 1421 is being used, so my tentative > >guess would be that gcc is picking an address register based on the > >constraints in addsi3_5200. Perhaps you need to change "?a" to "*a". > >After all, you probably don't want to encourage pseudos to go into the > >address registers merely because you add values to them. > > 1421 is only used in 5559 and 5560. from the lreg dump: > > Register 1421 used 2 times across 2 insns in block 694; set 1 time; pref > DATA_REGS. That is odd. Your earlier e-mail seemed to show that register 1421 was specifically put into an address register, specifically %a0. Why would that happen if it has a preference for DATA_REGS? Was it assigned by local-alloc? In that case you would see a line like ";; Register 2421 in NNN." in the .lreg dump file. > >In general, though, I'm not sure that prohibiting QImode values in > >address registers is going to be a useful approach. After all, the > >values do fit there, and while we want to discourage it, it's not > >obvious that we want to completely prohibit it. The problem in PR > >18421 is that reload decided that it had to reload a QImode value from > >memory into an address register, but it couldn't. That is more or > >less easy to handle by defining SECONDARY_RELOAD_CLASS to indicate > >that a data register is required to move QImode values between the > >address registers and memory, and by defining reload_inqi and > >reload_outqi to do the moves. > > The problem is that there is no valid QImode instruction that can move > values in/out of an address register.... I know. I'm suggesting that QImode values have to move in and out of address registers via data registers, so you just put the QImode value into the data register, and then move it into the address register, or vice-versa. But I see from your reference that Jim Wilson already suggested this, and that RTH advised against it. Hmmm. > Looking over the code, I see that PREFERRED_RELOAD_CLASS is: ... > Which looks like it allows QImode into ADDR_REGS instead > of insisting on DATA_REGS. Do you think this should be: No, that will break reload. If it calls PREFERRED_RELOAD_CLASS with ADDR_REGS, then it has already selected an alternative which requires ADDR_REGS. Returning a register class which does not permit any register in ADDR_REGS will give you a constraint violation later on. You could probably get this to work if you changed "?a" to "!a" in addsi3_5200. But that would probably give you worse code when dealing with SImode value. You could probably get this to work if you wrote the predicate for addsi3_5200 to specifically reject a subreg:SI of a QImode value, and then defined a new insn which specifically accepted it and nothing else, but did not permit address registers in the alternatives. You might have to play similar tricks with some other instructions, but perhaps addsi3 is relatively special here, just to handle cases generated by reload. Ian
