| Message ID | alpine.LFD.2.21.2012101617100.2104409@eddie.linux-mips.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | VAX: QMATH DImode add/sub fixes | expand |
On 12/11/20 1:48 PM, Maciej W. Rozycki wrote: > Handle constant 0 passed to the QMATH DImode add/sub handler such as > with: > > #2 0x0000000011d409b0 in gen_adddi3 (operand0=0x7ffff5c0a128, > operand1=0x7ffff5c60480, operand2=0x7ffff5c60470) > at .../gcc/config/vax/vax.md:755 > 755 "vax_expand_addsub_di_operands (operands, PLUS); DONE;") > (gdb) pr operand0 > (reg:DI 31) > (gdb) pr operand1 > (const_int 0 [0]) > (gdb) pr operand2 > (const_int -1 [0xffffffffffffffff]) > (gdb) > > causing an assertion in `vax_expand_addsub_di_operands': > > gcc_assert (operands[1] != const0_rtx || code == MINUS); > > to trigger: > > during RTL pass: expand > .../gcc/testsuite/gcc.c-torture/compile/sync-1.c: In function 'test_op_ignore': > .../gcc/testsuite/gcc.c-torture/compile/sync-1.c:33:10: internal compiler error: in vax_expand_addsub_di_operands, at config/vax/vax.c:2080 > 0x11815003 vax_expand_addsub_di_operands(rtx_def**, rtx_code) > .../gcc/config/vax/vax.c:2080 > 0x11d409af gen_adddi3(rtx_def*, rtx_def*, rtx_def*) > .../gcc/config/vax/vax.md:755 > 0x10ea2763 rtx_insn* insn_gen_fn::operator()<rtx_def*, rtx_def*, rtx_def*>(rtx_def*, rtx_def*, rtx_def*) const > .../gcc/recog.h:304 > 0x10f7fc8f maybe_gen_insn(insn_code, unsigned int, expand_operand*) > .../gcc/optabs.c:7402 > 0x10f67f8b expand_binop_directly > .../gcc/optabs.c:1122 > 0x10f684cf expand_binop(machine_mode, optab_tag, rtx_def*, rtx_def*, rtx_def*, int, optab_methods) > .../gcc/optabs.c:1209 > 0x10f6fb4f expand_unop(machine_mode, optab_tag, rtx_def*, rtx_def*, int) > .../gcc/optabs.c:3013 > 0x10f6c493 expand_simple_unop(machine_mode, rtx_code, rtx_def*, rtx_def*, int) > .../gcc/optabs.c:2200 > 0x10f7e2f3 expand_atomic_fetch_op(rtx_def*, rtx_def*, rtx_def*, rtx_code, memmodel, bool) > .../gcc/optabs.c:7021 > 0x107f7523 expand_builtin_sync_operation > .../gcc/builtins.c:7605 > 0x107ff547 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int) > .../gcc/builtins.c:9430 > 0x10acda63 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) > .../gcc/expr.c:11249 > 0x10abeb9f expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) > .../gcc/expr.c:8486 > 0x1085606b expand_expr > .../gcc/expr.h:282 > 0x1086157f expand_call_stmt > .../gcc/cfgexpand.c:2709 > 0x10865ab7 expand_gimple_stmt_1 > .../gcc/cfgexpand.c:3713 > 0x108662fb expand_gimple_stmt > .../gcc/cfgexpand.c:3877 > 0x10870387 expand_gimple_basic_block > .../gcc/cfgexpand.c:5918 > 0x10872b6b execute > .../gcc/cfgexpand.c:6602 > Please submit a full bug report, > with preprocessed source if appropriate. > Please include the complete backtrace with any bug report. > See <https://gcc.gnu.org/bugs/> for instructions. > compiler exited with status 1 > FAIL: gcc.c-torture/compile/sync-1.c -O0 (internal compiler error) > > causing numerous failures in regression testing. > > While requesting an addition operation to be produced for the constant > operands of 0 and -1 may seem silly, technically there is nothing wrong > with it, and non-QMATH code (as with the `-mno-qmath' option) has no > issues with that, so neither should QMATH code. This operation will > normally be folded in later passes anyway. > > Observe then, that adding or subtracting constant 0 amounts to a move > (and we even have a machine instruction available to do that with a > single operation) so handle the case explicitly, swapping the addends if > so required, removing the assertion failure and along with that 70 test > suite failures like: > > FAIL: gcc.c-torture/compile/sync-1.c -O0 (internal compiler error) > FAIL: gcc.c-torture/compile/sync-1.c -O0 fetch_and_nand (test for warnings, line ) > FAIL: gcc.c-torture/compile/sync-1.c -O0 nand_and_fetch (test for warnings, line ) > FAIL: gcc.c-torture/compile/sync-1.c -O0 (test for excess errors) > FAIL: gcc.c-torture/compile/sync-2.c -O0 (internal compiler error) > FAIL: gcc.c-torture/compile/sync-2.c -O0 (test for warnings, line ) > FAIL: gcc.c-torture/compile/sync-2.c -O0 (test for excess errors) > FAIL: gcc.c-torture/compile/sync-3.c -O0 (internal compiler error) > FAIL: gcc.c-torture/compile/sync-3.c -O0 (test for warnings, line ) > FAIL: gcc.c-torture/compile/sync-3.c -O0 (test for excess errors) > > and similarly across all the other optimization levels and compilation > options covered. > > gcc/ > * config/vax/vax.c (vax_expand_addsub_di_operands): Handle the > addition or subtraction of 0. OK, though I would have generally expected something to catch this earlier. jeff
On Sun, 13 Dec 2020, Jeff Law wrote: > > gcc/ > > * config/vax/vax.c (vax_expand_addsub_di_operands): Handle the > > addition or subtraction of 0. > OK, though I would have generally expected something to catch this earlier. Doesn't it matter that this pattern is produced in the context of atomic intrinsics? It is also not invalid even if silly and the !QMATH VAX implementation (the older one, presumably) lets it through just fine. Last but not least the operation does not make it to assembly output; I could perhaps trace it through the RTL passes, but so far I have decided it doesn't really matter which specific pass makes it disappear. Either way thanks for your review, I have pushed this and the remaining five changes. Maciej
Index: gcc/gcc/config/vax/vax.c =================================================================== --- gcc.orig/gcc/config/vax/vax.c +++ gcc/gcc/config/vax/vax.c @@ -2066,6 +2066,19 @@ vax_expand_addsub_di_operands (rtx * ope else operands[2] = fixup_mathdi_operand (operands[2], DImode); + /* If we are adding or subtracting 0, then this is a move. */ + if (code == PLUS && operands[1] == const0_rtx) + { + temp = operands[2]; + operands[2] = operands[1]; + operands[1] = temp; + } + if (operands[2] == const0_rtx) + { + emit_move_insn (operands[0], operands[1]); + return; + } + /* If we are subtracting not from ourselves [d = a - b], and because the carry ops are two operand only, we would need to do a move prior to the subtract. And if d == b, we would need a temp otherwise
Handle constant 0 passed to the QMATH DImode add/sub handler such as with: #2 0x0000000011d409b0 in gen_adddi3 (operand0=0x7ffff5c0a128, operand1=0x7ffff5c60480, operand2=0x7ffff5c60470) at .../gcc/config/vax/vax.md:755 755 "vax_expand_addsub_di_operands (operands, PLUS); DONE;") (gdb) pr operand0 (reg:DI 31) (gdb) pr operand1 (const_int 0 [0]) (gdb) pr operand2 (const_int -1 [0xffffffffffffffff]) (gdb) causing an assertion in `vax_expand_addsub_di_operands': gcc_assert (operands[1] != const0_rtx || code == MINUS); to trigger: during RTL pass: expand .../gcc/testsuite/gcc.c-torture/compile/sync-1.c: In function 'test_op_ignore': .../gcc/testsuite/gcc.c-torture/compile/sync-1.c:33:10: internal compiler error: in vax_expand_addsub_di_operands, at config/vax/vax.c:2080 0x11815003 vax_expand_addsub_di_operands(rtx_def**, rtx_code) .../gcc/config/vax/vax.c:2080 0x11d409af gen_adddi3(rtx_def*, rtx_def*, rtx_def*) .../gcc/config/vax/vax.md:755 0x10ea2763 rtx_insn* insn_gen_fn::operator()<rtx_def*, rtx_def*, rtx_def*>(rtx_def*, rtx_def*, rtx_def*) const .../gcc/recog.h:304 0x10f7fc8f maybe_gen_insn(insn_code, unsigned int, expand_operand*) .../gcc/optabs.c:7402 0x10f67f8b expand_binop_directly .../gcc/optabs.c:1122 0x10f684cf expand_binop(machine_mode, optab_tag, rtx_def*, rtx_def*, rtx_def*, int, optab_methods) .../gcc/optabs.c:1209 0x10f6fb4f expand_unop(machine_mode, optab_tag, rtx_def*, rtx_def*, int) .../gcc/optabs.c:3013 0x10f6c493 expand_simple_unop(machine_mode, rtx_code, rtx_def*, rtx_def*, int) .../gcc/optabs.c:2200 0x10f7e2f3 expand_atomic_fetch_op(rtx_def*, rtx_def*, rtx_def*, rtx_code, memmodel, bool) .../gcc/optabs.c:7021 0x107f7523 expand_builtin_sync_operation .../gcc/builtins.c:7605 0x107ff547 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int) .../gcc/builtins.c:9430 0x10acda63 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) .../gcc/expr.c:11249 0x10abeb9f expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) .../gcc/expr.c:8486 0x1085606b expand_expr .../gcc/expr.h:282 0x1086157f expand_call_stmt .../gcc/cfgexpand.c:2709 0x10865ab7 expand_gimple_stmt_1 .../gcc/cfgexpand.c:3713 0x108662fb expand_gimple_stmt .../gcc/cfgexpand.c:3877 0x10870387 expand_gimple_basic_block .../gcc/cfgexpand.c:5918 0x10872b6b execute .../gcc/cfgexpand.c:6602 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. compiler exited with status 1 FAIL: gcc.c-torture/compile/sync-1.c -O0 (internal compiler error) causing numerous failures in regression testing. While requesting an addition operation to be produced for the constant operands of 0 and -1 may seem silly, technically there is nothing wrong with it, and non-QMATH code (as with the `-mno-qmath' option) has no issues with that, so neither should QMATH code. This operation will normally be folded in later passes anyway. Observe then, that adding or subtracting constant 0 amounts to a move (and we even have a machine instruction available to do that with a single operation) so handle the case explicitly, swapping the addends if so required, removing the assertion failure and along with that 70 test suite failures like: FAIL: gcc.c-torture/compile/sync-1.c -O0 (internal compiler error) FAIL: gcc.c-torture/compile/sync-1.c -O0 fetch_and_nand (test for warnings, line ) FAIL: gcc.c-torture/compile/sync-1.c -O0 nand_and_fetch (test for warnings, line ) FAIL: gcc.c-torture/compile/sync-1.c -O0 (test for excess errors) FAIL: gcc.c-torture/compile/sync-2.c -O0 (internal compiler error) FAIL: gcc.c-torture/compile/sync-2.c -O0 (test for warnings, line ) FAIL: gcc.c-torture/compile/sync-2.c -O0 (test for excess errors) FAIL: gcc.c-torture/compile/sync-3.c -O0 (internal compiler error) FAIL: gcc.c-torture/compile/sync-3.c -O0 (test for warnings, line ) FAIL: gcc.c-torture/compile/sync-3.c -O0 (test for excess errors) and similarly across all the other optimization levels and compilation options covered. gcc/ * config/vax/vax.c (vax_expand_addsub_di_operands): Handle the addition or subtraction of 0. --- gcc/config/vax/vax.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)