On Mon, 2013-08-19 at 16:49 -0400, DJ Delorie wrote: > > I'd say it's not as simple as you make it out to be. You can't blindly > > combine operations on volatile memory. > > I'm not blindly combining them, I'm combining them when I know the > hardware will do the volatile-correct thing. > > > ie, the programmer may have written them as separate statements and > > if they're volatile you should leave them alone. > > Most of the programmers I know would expect "port |= 0x80;" to do a > hardware-specific "set bits" operation, not a series of > volatile-pedantic operations, especially when the ISA has a set of > insns specifically designed to do bit operations on volatile I/O > registers. > > > With this change a series of statements involving > > volatiles might be combined into a single insn. That's not good. > > If the insn does exactly the same thing as the operations, just > faster, why is it not good? > > > I'll note that *NO* ports in the official GCC sources do this and that's > > because it's wrong. > > m32c, rl78, and sh do this (to varying degrees) in the official > sources.
Just a note regarding the issue in SH: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52483 In this case, the problem is that the volatile handling in 'general_operand' prevents matching of 'complex' addresses and sign extending mem loads. The recent SH volatile mem load fix doesn't play with the 'volatile_ok' variable before invoking 'general_operand', but rather avoids it altogether, by not using 'general_operand' for matching mems. In this case it should be fine, since it's done in the predicate 'general_movsrc_operand' which is used for mem loads only and not in arithmetic insns. Maybe it would be better to handle this in a general way somehow instead of having to do workarounds in each target. Cheers, Oleg
