Hi! On Tue, Jul 14, 2020 at 03:26:50PM +0200, Jakub Jelinek wrote: > On Tue, Jul 14, 2020 at 02:33:41PM +0200, Richard Biener wrote: > > > As mentioned in the PR, we generate a useless movzbl insn before lock > > > cmpxchg. > > > The problem is that the builtin for the char/short cases has the arguments > > > promoted to int and combine gives up, because the instructions have > > > MEM_VOLATILE_P arguments and recog in that case doesn't recognize anything > > > when volatile_ok is false, and nothing afterwards optimizes the > > > (reg:SI a) = (zero_extend:SI (reg:QI a)) > > > ... (subreg:QI (reg:SI a) 0) ... > > > > So the above isn't fixable? Because it would probably be the more > > generic fix, right? > > I'm afraid it is not, CCing Segher on that. The question is how to > differentiate between "combine didn't do anything dangerous to this > MEM_VOLATILE_P and just kept it in the pattern as is" vs. > "combine changed it in a dangerous way unsuitable for volatile accesses".
This can be handled by having a predicate that allows volatile memory. Various targets do this already (rs6000 is the most prominent). You then have to make sure that every insn using one of those predicates actually *does* do everything volatile memory needs to be done ;-) If ever variable is aligned (say the ABI requires that), you are 98% there already. If not, it is much harder. > Even if we wanted to make just the case where i3 is the only insn > that originally contains MEM_VOLATILE_P and checked that the MEM stayed as > is, there is the difficulty that we change the insns in place, so remembering > how it looked before is hard. And then before trying to recognize it we'd > carefully need to check that nothing else is volatile before enabling > temporarily volatile_ok. Yeah, don't, this wont't work :-) Segher
