> This is a target-specific blockage insn, but with the general form > found in all targets defining it. The default blockage is an empty > asm-volatile, which is what cse_insn recognized. The blockage insn is > there to "prevent scheduling" of the critical insns and register > values. It's almost obvious that an unspec_volatile should have that > effect "too"; at least that's intended by the code in > expand_builtin_setjmp_receiver. Luckily (or unluckily regarding the > presence of the bug) *this* cse code is not run post-frame-layout > (post-reload-cse does not use this code), or it seems people would > soon notice register values used from the wrong side of the blockage, > considering its critical use at the restore of the stack-pointer. > As mentioned in the previous patch, > <http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01172.html>, clobbering > the frame-pointer (and then using it) does not seem the logical > alternative to the patch below; the blockage insn should just do its job.
Agreed. > I updated comments and documentation so it's more apparent that > register uses should also not be moved across the blockage; see the > patched comments. > > Tested native x86_64-unknown-linux-gnu w./wo. -m32 and before/after > 192677. Ok to commit? > > gcc: > PR middle-end/55030 > * builtins.c (expand_builtin_setjmp_receiver): Update comment > regarding purpose of blockage. > * emit-rtl.c [!HAVE_blockage] (gen_blockage): Similarly for > the head comment. > * doc/md.texi (blockage): Update similarly. Change wording to > require one of two forms, rather than implying a wider choice. > * cse.c (cse_insn): Where checking for blocking insns, treat > UNSPEC_VOLATILE as blocking, besides volatile ASM. That's fine on principle, but there is a predicate for this (volatile_insn_p) so I think we should use it here. Moreover, cselib_process_insn has the same check so we should adjust it as well, which in turn means that dse.c:scan_insn should probably be adjusted too. OK with these changes, thanks. -- Eric Botcazou
