Re: syncing the GCC vax port, atomic issue

[Jeff Law <law@redhat.com>]

On 9/20/19 3:04 PM, coypu@sdf.org wrote:
> On Fri, Sep 20, 2019 at 11:15:32AM +0000, coypu@sdf.org wrote:
>> Removed from the diff. Unfortunately this introduces an ICE during the
>> build of GCC...
> 
> I took another look at the VAX atomic pattern issue.
> (http://gnats.netbsd.org/53039)
> It is a compiler crash to do with the added atomic builtins.
> 
> The original suggestion was to introduce a reversed pattern, with
> label / pc swapped. It did not work, unfortunately.
> 
> The crash backtrace is (gcc-trunk line numbers):
> during RTL pass: expand
> /home/fly/gcc/libatomic/flag.c: In function 'atomic_flag_test_and_set':
> /home/fly/gcc/libatomic/flag.c:36:1: internal compiler error: in patch_jump_insn, at cfgrtl.c:1291
>    36 | }
>       | ^
> 0x718c22 patch_jump_insn
>         /home/fly/gcc/gcc/cfgrtl.c:1290
> 0x718d2f redirect_branch_edge
>         /home/fly/gcc/gcc/cfgrtl.c:1317
> 0x71b8c2 rtl_redirect_edge_and_branch
>         /home/fly/gcc/gcc/cfgrtl.c:1450
> 0x703909 redirect_edge_and_branch(edge_def*, basic_block_def*)
>         /home/fly/gcc/gcc/cfghooks.c:373
> 0x119ec6e try_forward_edges
>         /home/fly/gcc/gcc/cfgcleanup.c:558
> 0x11a26ca try_optimize_cfg
>         /home/fly/gcc/gcc/cfgcleanup.c:2961
> 0x11a26ca cleanup_cfg(int)
>         /home/fly/gcc/gcc/cfgcleanup.c:3175
> 0x700a41 execute
>         /home/fly/gcc/gcc/cfgexpand.c:6683
> 
> 
> This seems to be where GCC has some logic specific to optimizing jumps.
> Since this isn't a normal jump possible to eliminate by being clever,
> but rather our only way of doing atomics, my gut feeling is that I
> should avoid this jump optimizing code entirely.
> 
> Not telling GCC it's a jump worth optimizing seems to avoid the crash,
> i.e.:
> 
> -  emit_jump_insn (gen_jbbssi<mode> (operands[1], const0_rtx, label, operands[1]));
> +  emit_insn (gen_jbbssi<mode> (operands[1], const0_rtx, label, operands[1]));
> +  LABEL_NUSES (label)++;
> 
> GCC builds, but when building netbsd I get an undefined reference in
> libstdc++:
> libstdc++.so: undefined reference to `.L72'
> libstdc++.so: undefined reference to `.L75'
> 
> I wonder whether this is a right approach with a slightly off method.
Conditional branching patterns must support the label_ref and pc
operands in either position.  Everything else I've seen on this thread
is just working around that broken aspect of the builtins.md file.


(define_insn "jbbssiqi"
  [(parallel
    [(set (pc)
	  (if_then_else
	    (ne (zero_extract:SI (match_operand:QI 0 "memory_operand" "g")
				 (const_int 1)
				 (match_operand:SI 1 "general_operand" "nrm"))
		(const_int 0))
	    (label_ref (match_operand 2 "" ""))
	    (pc)))
     (set (zero_extract:SI (match_operand:QI 3 "memory_operand" "+0")
			   (const_int 1)
			   (match_dup 1))
	  (const_int 1))])]
  ""
  "jbssi %1,%0,%l2")

Note the position of the (label_ref ...) and (pc) operand.  You have to
have a pattern where they are reversed as well.

As an example look at the branch and branch_reversed patterns in vax.md

jeff

>