| Message ID | alpine.LFD.2.21.2011200244300.656242@eddie.linux-mips.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | VAX: Bring the port up to date (yes, MODE_CC conversion is included) | expand |
On 11/19/20 8:34 PM, Maciej W. Rozycki wrote: > VAX has interlocked branch instructions used for atomic operations and > we want to have them wrapped in UNSPEC_VOLATILE so as not to have code > carried across. This however breaks with jump optimization and leads > to an ICE in the build of libbacktrace like: > > .../libbacktrace/mmap.c:190:1: internal compiler error: in fixup_reorder_chain, at cfgrtl.c:3934 > 190 | } > | ^ > 0x1087d46b fixup_reorder_chain > .../gcc/cfgrtl.c:3934 > 0x1087f29f cfg_layout_finalize() > .../gcc/cfgrtl.c:4447 > 0x1087c74f execute > .../gcc/cfgrtl.c:3662 > > on RTL like: > > (jump_insn 18 17 150 4 (unspec_volatile [ > (set (pc) > (if_then_else (eq (zero_extract:SI (mem/v:SI (reg/f:SI 23 [ _2 ]) [-1 S4 A32]) > (const_int 1 [0x1]) > (const_int 0 [0])) > (const_int 1 [0x1])) > (label_ref 20) > (pc))) > (set (zero_extract:SI (mem/v:SI (reg/f:SI 23 [ _2 ]) [-1 S4 A32]) > (const_int 1 [0x1]) > (const_int 0 [0])) > (const_int 1 [0x1])) > ] 101) ".../libbacktrace/mmap.c":135:14 158 {jbbssisi} > (nil) > -> 20) > > when those branches are enabled with a follow-up change. Also showing > with: > > FAIL: gcc.dg/pr61756.c (internal compiler error) > > Handle branches wrapped in UNSPEC_VOLATILE then and, for consistency, > also in UNSPEC. The presence of UNSPEC_VOLATILE will prevent such > branches from being removed as they won't be accepted by `onlyjump_p', > we just need to let them through. > > gcc/ > * jump.c (pc_set): Also accept a jump wrapped in UNSPEC or > UNSPEC_VOLATILE. > (any_uncondjump_p, any_condjump_p): Update comment accordingly. I've got some concerns that there may be users of pc_set where handling UNSPECs would be undesirable. For example the uses in cfgcleanup. Would it make sense to have the handling of UNSPECs be conditional so that we don't get unexpected jump threading, cross jumping, etc? jeff
diff --git a/gcc/jump.c b/gcc/jump.c index 34a8f209e20..f4c735540f0 100644 --- a/gcc/jump.c +++ b/gcc/jump.c @@ -850,9 +850,17 @@ pc_set (const rtx_insn *insn) pat = PATTERN (insn); /* The set is allowed to appear either as the insn pattern or - the first set in a PARALLEL. */ - if (GET_CODE (pat) == PARALLEL) - pat = XVECEXP (pat, 0, 0); + the first set in a PARALLEL, UNSPEC or UNSPEC_VOLATILE. */ + switch (GET_CODE (pat)) + { + case PARALLEL: + case UNSPEC: + case UNSPEC_VOLATILE: + pat = XVECEXP (pat, 0, 0); + break; + default: + break; + } if (GET_CODE (pat) == SET && GET_CODE (SET_DEST (pat)) == PC) return pat; @@ -860,7 +868,7 @@ pc_set (const rtx_insn *insn) } /* Return true when insn is an unconditional direct jump, - possibly bundled inside a PARALLEL. */ + possibly bundled inside a PARALLEL, UNSPEC or UNSPEC_VOLATILE. */ int any_uncondjump_p (const rtx_insn *insn) @@ -876,9 +884,9 @@ any_uncondjump_p (const rtx_insn *insn) } /* Return true when insn is a conditional jump. This function works for - instructions containing PC sets in PARALLELs. The instruction may have - various other effects so before removing the jump you must verify - onlyjump_p. + instructions containing PC sets in PARALLELs, UNSPECs or UNSPEC_VOLATILEs. + The instruction may have various other effects so before removing the jump + you must verify onlyjump_p. Note that unlike condjump_p it returns false for unconditional jumps. */
VAX has interlocked branch instructions used for atomic operations and we want to have them wrapped in UNSPEC_VOLATILE so as not to have code carried across. This however breaks with jump optimization and leads to an ICE in the build of libbacktrace like: .../libbacktrace/mmap.c:190:1: internal compiler error: in fixup_reorder_chain, at cfgrtl.c:3934 190 | } | ^ 0x1087d46b fixup_reorder_chain .../gcc/cfgrtl.c:3934 0x1087f29f cfg_layout_finalize() .../gcc/cfgrtl.c:4447 0x1087c74f execute .../gcc/cfgrtl.c:3662 on RTL like: (jump_insn 18 17 150 4 (unspec_volatile [ (set (pc) (if_then_else (eq (zero_extract:SI (mem/v:SI (reg/f:SI 23 [ _2 ]) [-1 S4 A32]) (const_int 1 [0x1]) (const_int 0 [0])) (const_int 1 [0x1])) (label_ref 20) (pc))) (set (zero_extract:SI (mem/v:SI (reg/f:SI 23 [ _2 ]) [-1 S4 A32]) (const_int 1 [0x1]) (const_int 0 [0])) (const_int 1 [0x1])) ] 101) ".../libbacktrace/mmap.c":135:14 158 {jbbssisi} (nil) -> 20) when those branches are enabled with a follow-up change. Also showing with: FAIL: gcc.dg/pr61756.c (internal compiler error) Handle branches wrapped in UNSPEC_VOLATILE then and, for consistency, also in UNSPEC. The presence of UNSPEC_VOLATILE will prevent such branches from being removed as they won't be accepted by `onlyjump_p', we just need to let them through. gcc/ * jump.c (pc_set): Also accept a jump wrapped in UNSPEC or UNSPEC_VOLATILE. (any_uncondjump_p, any_condjump_p): Update comment accordingly. --- gcc/jump.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)