Message ID | alpine.LFD.2.21.2011200255430.656242@eddie.linux-mips.org |
---|---|
State | Accepted |
Headers | show |
Series | VAX: Bring the port up to date (yes, MODE_CC conversion is included) | expand |
On 11/19/20 8:35 PM, Maciej W. Rozycki wrote: > The INSV machine instruction is the only computational operation in the > VAX ISA that keeps condition codes intact. In preparation to MODE_CC > transition keep patterns apart then that make or do not make use of said > instruction. For consistency update EXTV and EXTZV instruction uses > accordingly. In expand SUBREGs will be presented as operands, so handle > that possibility in the insn condition. > > This actually yields better code by avoiding EXTV/EXTZV instructions in > pseudo-aligned register cases previously resorting to those instructions: > > @@ -42,7 +42,7 @@ ins8: > subl2 $4,%sp # 21 [c=32] addsi3 > movl 4(%ap),%r0 # 2 [c=16] movsi_2 > movl 8(%ap),%r1 # 17 [c=16] movsi_2 > - insv %r1,$8,$8,%r0 # 9 [c=4] *insv_aligned > + insv %r1,$8,$8,%r0 # 9 [c=4] *insv_2 > ret # 25 [c=0] return > .size ins8, .-ins8 > .align 1 > @@ -60,12 +60,12 @@ ext8: > .globl extz8 > .type extz8, @function > extz8: > - .word 0 # 19 [c=0] procedure_entry_mask > - subl2 $4,%sp # 20 [c=32] addsi3 > + .word 0 # 18 [c=0] procedure_entry_mask > + subl2 $4,%sp # 19 [c=32] addsi3 > movl 4(%ap),%r0 # 2 [c=16] movsi_2 > - extzv $8,$8,%r0,%r1 # 13 [c=60] *extzv_aligned > - movl %r1,%r0 # 18 [c=4] movsi_2 > - ret # 24 [c=0] return > + rotl $24,%r0,%r0 # 13 [c=60] *extzv_non_const > + movzbl %r0,%r0 > + ret # 23 [c=0] return > .size extz8, .-extz8 > .align 1 > .globl ins16 > @@ -75,7 +75,7 @@ ins16: > subl2 $4,%sp # 21 [c=32] addsi3 > movl 4(%ap),%r0 # 2 [c=16] movsi_2 > movl 8(%ap),%r1 # 17 [c=16] movsi_2 > - insv %r1,$16,$16,%r0 # 9 [c=4] *insv_aligned > + insv %r1,$16,$16,%r0 # 9 [c=4] *insv_2 > ret # 25 [c=0] return > .size ins16, .-ins16 > .align 1 > @@ -94,8 +94,9 @@ ext16: > extz16: > .word 0 # 18 [c=0] procedure_entry_mask > subl2 $4,%sp # 19 [c=32] addsi3 > - movl 4(%ap),%r1 # 2 [c=16] movsi_2 > - extzv $16,$16,%r1,%r0 # 7 [c=60] *extzv_aligned > + movl 4(%ap),%r0 # 2 [c=16] movsi_2 > + rotl $16,%r0,%r0 # 7 [c=60] *extzv_non_const > + movzwl %r0,%r0 > movzwl %r0,%r0 # 13 [c=4] zero_extendhisi2 > ret # 23 [c=0] return > .size extz16, .-extz16 > > demonstrated with this program: > > typedef struct > { > int f0:1; > int f1:7; > int f8:8; > int f16:16; > } bit_t; > > typedef struct > { > unsigned int f0:1; > unsigned int f1:7; > unsigned int f8:8; > unsigned int f16:16; > } ubit_t; > > typedef union > { > bit_t b; > int i; > } bit_u; > > typedef union > { > ubit_t b; > unsigned int i; > } ubit_u; > > int > ins1 (bit_u x, int y) > { > asm volatile ("" : "+r" (x), "+r" (y)); > x.b.f1 = y; > return x.i; > } > > int > ext1 (bit_u x) > { > asm volatile ("" : "+r" (x)); > return x.b.f1; > } > > unsigned int > extz1 (ubit_u x) > { > asm volatile ("" : "+r" (x)); > return x.b.f1; > } > > int > ins8 (bit_u x, int y) > { > asm volatile ("" : "+r" (x), "+r" (y)); > x.b.f8 = y; > return x.i; > } > > int > ext8 (bit_u x) > { > asm volatile ("" : "+r" (x)); > return x.b.f8; > } > > unsigned int > extz8 (ubit_u x) > { > asm volatile ("" : "+r" (x)); > return x.b.f8; > } > > int > ins16 (bit_u x, int y) > { > asm volatile ("" : "+r" (x), "+r" (y)); > x.b.f16 = y; > return x.i; > } > > int > ext16 (bit_u x) > { > asm volatile ("" : "+r" (x)); > return x.b.f16; > } > > unsigned int > extz16 (ubit_u x) > { > asm volatile ("" : "+r" (x)); > return x.b.f16; > } > > It also papers over a regression: > > FAIL: gcc.dg/pr83623.c (internal compiler error) > FAIL: gcc.dg/pr83623.c (test for excess errors) > > from an ICE like: > > during RTL pass: final > .../gcc/testsuite/gcc.dg/pr83623.c: In function 'foo': > .../gcc/testsuite/gcc.dg/pr83623.c:13:1: internal compiler error: in change_address_1, at emit-rtl.c:2275 > 0x10a056e3 change_address_1 > .../gcc/emit-rtl.c:2275 > 0x10a0645f adjust_address_1(rtx_def*, machine_mode, poly_int<1u, long>, int, int, int, poly_int<1u, long>) > .../gcc/emit-rtl.c:2409 > 0x11cb588f output_97 > .../gcc/config/vax/vax.md:808 > 0x10aafb2f get_insn_template(int, rtx_insn*) > .../gcc/final.c:2070 > 0x10ab2b3f final_scan_insn_1 > .../gcc/final.c:3039 > 0x10ab313b final_scan_insn(rtx_insn*, _IO_FILE*, int, int, int*) > .../gcc/final.c:3152 > 0x10aaf887 final_1 > .../gcc/final.c:2020 > 0x10ab703b rest_of_handle_final > .../gcc/final.c:4658 > 0x10ab757b execute > .../gcc/final.c:4736 > 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.dg/pr83623.c (internal compiler error) > > triggered by an RTL instruction like: > > (insn 17 14 145 (set (reg:SI 1 %r1) > (zero_extract:SI (mem/c:SI (symbol_ref:SI ("x") <var_decl 0x7ffff7f80120 x>) [1 x+0 S4 A128]) > (const_int 16 [0x10]) > (const_int 16 [0x10]))) ".../gcc/testsuite/gcc.dg/pr83623.c":12:9 97 {*extzv_aligned} > (nil)) > > (where the address cannot be adjusted by 2 for PIC code as requested > here as it would create an offset external symbol reference) otherwise > caused by the patterns modified here, addressed next. This indicates > a further rework is warranted here, but at least problems at hand have > been fixed. > > gcc/ > * config/vax/vax.md (*insv_aligned, *extzv_aligned) > (*extv_aligned): Reject register bitfield locations that are not > aligned to the least significant bit; update output statement > accordingly. OK jeff
diff --git a/gcc/config/vax/vax.md b/gcc/config/vax/vax.md index de90848a600..80f09d97727 100644 --- a/gcc/config/vax/vax.md +++ b/gcc/config/vax/vax.md @@ -754,8 +754,8 @@ (define_insn "" ;; Special cases of bit-field insns which we should ;; recognize in preference to the general case. -;; These handle aligned 8-bit and 16-bit fields, -;; which can usually be done with move instructions. +;; These handle aligned 8-bit and 16-bit fields +;; that can be done with move or convert instructions. (define_insn "*insv_aligned" [(set (zero_extract:SI (match_operand:SI 0 "nonimmediate_operand" "+ro") @@ -766,19 +766,19 @@ (define_insn "*insv_aligned" && INTVAL (operands[2]) % INTVAL (operands[1]) == 0 && (!MEM_P (operands[0]) || ! mode_dependent_address_p (XEXP (operands[0], 0), - MEM_ADDR_SPACE (operands[0])))" + MEM_ADDR_SPACE (operands[0]))) + && (!(REG_P (operands[0]) + || (SUBREG_P (operands[0]) && REG_P (SUBREG_REG (operands[0])))) + || INTVAL (operands[2]) == 0)" "* { - if (REG_P (operands[0])) - { - if (INTVAL (operands[2]) != 0) - return \"insv %3,%2,%1,%0\"; - } - else + if (!REG_P (operands[0])) operands[0] = adjust_address (operands[0], INTVAL (operands[1]) == 8 ? QImode : HImode, INTVAL (operands[2]) / 8); + else + gcc_assert (INTVAL (operands[2]) == 0); CC_STATUS_INIT; if (INTVAL (operands[1]) == 8) @@ -795,19 +795,19 @@ (define_insn "*extzv_aligned" && INTVAL (operands[3]) % INTVAL (operands[2]) == 0 && (!MEM_P (operands[1]) || ! mode_dependent_address_p (XEXP (operands[1], 0), - MEM_ADDR_SPACE (operands[1])))" + MEM_ADDR_SPACE (operands[1]))) + && (!(REG_P (operands[1]) + || (SUBREG_P (operands[1]) && REG_P (SUBREG_REG (operands[1])))) + || INTVAL (operands[3]) == 0)" "* { - if (REG_P (operands[1])) - { - if (INTVAL (operands[3]) != 0) - return \"extzv %3,%2,%1,%0\"; - } - else + if (!REG_P (operands[1])) operands[1] = adjust_address (operands[1], INTVAL (operands[2]) == 8 ? QImode : HImode, INTVAL (operands[3]) / 8); + else + gcc_assert (INTVAL (operands[3]) == 0); if (INTVAL (operands[2]) == 8) return \"movzbl %1,%0\"; @@ -823,19 +823,19 @@ (define_insn "*extv_aligned" && INTVAL (operands[3]) % INTVAL (operands[2]) == 0 && (!MEM_P (operands[1]) || ! mode_dependent_address_p (XEXP (operands[1], 0), - MEM_ADDR_SPACE (operands[1])))" + MEM_ADDR_SPACE (operands[1]))) + && (!(REG_P (operands[1]) + || (SUBREG_P (operands[1]) && REG_P (SUBREG_REG (operands[1])))) + || INTVAL (operands[3]) == 0)" "* { - if (REG_P (operands[1])) - { - if (INTVAL (operands[3]) != 0) - return \"extv %3,%2,%1,%0\"; - } - else + if (!REG_P (operands[1])) operands[1] = adjust_address (operands[1], INTVAL (operands[2]) == 8 ? QImode : HImode, INTVAL (operands[3]) / 8); + else + gcc_assert (INTVAL (operands[3]) == 0); if (INTVAL (operands[2]) == 8) return \"cvtbl %1,%0\";