Re: [RFA:] fix PR55030, wrong code from __builtin_setjmp_receiver

[Eric Botcazou <ebotcazou@adacore.com>]

> 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