This patch reattempts to change the ZERO_EXTRACTs and SIGN_EXTRACTs in i386.md to consistently use QImode for bit offsets (i.e. third and fourth operands), matching the use of QImode for bit counts in shifts and rotates.
This iteration corrects the "ne:QI" vs "eq:QI" mistake in the previous version, which was responsible for PR 110787 and PR 110790 and so was rapidly reverted last weekend. New test cases have been added to check the correct behaviour. This patch has been tested on x86_64-pc-linux-gnu with and without --enable-languages="all", with make bootstrap and make -k check, both with and without --target_board=unix{-m32} with no new failures. Committed to mainline as an obvious fix to the previously approved patch. Sorry again for the temporary inconvenience, and thanks to Rainer Orth for identifying/confirming the problematic patch. 2023-07-29 Roger Sayle <ro...@nextmovesoftware.com> gcc/ChangeLog PR target/110790 * config/i386/i386.md (extv<mode>): Use QImode for offsets. (extzv<mode>): Likewise. (insv<mode>): Likewise. (*testqi_ext_3): Likewise. (*btr<mode>_2): Likewise. (define_split): Likewise. (*btsq_imm): Likewise. (*btrq_imm): Likewise. (*btcq_imm): Likewise. (define_peephole2 x3): Likewise. (*bt<mode>): Likewise (*bt<mode>_mask): New define_insn_and_split. (*jcc_bt<mode>): Use QImode for offsets. (*jcc_bt<mode>_1): Delete obsolete pattern. (*jcc_bt<mode>_mask): Use QImode offsets. (*jcc_bt<mode>_mask_1): Likewise. (define_split): Likewise. (*bt<mode>_setcqi): Likewise. (*bt<mode>_setncqi): Likewise. (*bt<mode>_setnc<mode>): Likewise. (*bt<mode>_setncqi_2): Likewise. (*bt<mode>_setc<mode>_mask): New define_insn_and_split. (bmi2_bzhi_<mode>3): Use QImode offsets. (*bmi2_bzhi_<mode>3): Likewise. (*bmi2_bzhi_<mode>3_1): Likewise. (*bmi2_bzhi_<mode>3_1_ccz): Likewise. (@tbm_bextri_<mode>): Likewise. gcc/testsuite/ChangeLog PR target/110790 * gcc.target/i386/pr110790-1.c: New test case. * gcc.target/i386/pr110790-2.c: Likewise.
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 4db210c..efac228 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -3312,8 +3312,8 @@ (define_expand "extv<mode>" [(set (match_operand:SWI24 0 "register_operand") (sign_extract:SWI24 (match_operand:SWI24 1 "register_operand") - (match_operand:SI 2 "const_int_operand") - (match_operand:SI 3 "const_int_operand")))] + (match_operand:QI 2 "const_int_operand") + (match_operand:QI 3 "const_int_operand")))] "" { /* Handle extractions from %ah et al. */ @@ -3340,8 +3340,8 @@ (define_expand "extzv<mode>" [(set (match_operand:SWI248 0 "register_operand") (zero_extract:SWI248 (match_operand:SWI248 1 "register_operand") - (match_operand:SI 2 "const_int_operand") - (match_operand:SI 3 "const_int_operand")))] + (match_operand:QI 2 "const_int_operand") + (match_operand:QI 3 "const_int_operand")))] "" { if (ix86_expand_pextr (operands)) @@ -3428,8 +3428,8 @@ (define_expand "insv<mode>" [(set (zero_extract:SWI248 (match_operand:SWI248 0 "register_operand") - (match_operand:SI 1 "const_int_operand") - (match_operand:SI 2 "const_int_operand")) + (match_operand:QI 1 "const_int_operand") + (match_operand:QI 2 "const_int_operand")) (match_operand:SWI248 3 "register_operand"))] "" { @@ -10788,8 +10788,8 @@ (match_operator 1 "compare_operator" [(zero_extract:SWI248 (match_operand 2 "int_nonimmediate_operand" "rm") - (match_operand 3 "const_int_operand") - (match_operand 4 "const_int_operand")) + (match_operand:QI 3 "const_int_operand") + (match_operand:QI 4 "const_int_operand")) (const_int 0)]))] "/* Ensure that resulting mask is zero or sign extended operand. */ INTVAL (operands[4]) >= 0 @@ -15965,7 +15965,7 @@ [(set (zero_extract:HI (match_operand:SWI12 0 "nonimmediate_operand") (const_int 1) - (zero_extend:SI (match_operand:QI 1 "register_operand"))) + (match_operand:QI 1 "register_operand")) (const_int 0)) (clobber (reg:CC FLAGS_REG))] "TARGET_USE_BT && ix86_pre_reload_split ()" @@ -15989,7 +15989,7 @@ [(set (zero_extract:HI (match_operand:SWI12 0 "register_operand") (const_int 1) - (zero_extend:SI (match_operand:QI 1 "register_operand"))) + (match_operand:QI 1 "register_operand")) (const_int 0)) (clobber (reg:CC FLAGS_REG))] "TARGET_USE_BT && ix86_pre_reload_split ()" @@ -16016,7 +16016,7 @@ (define_insn "*btsq_imm" [(set (zero_extract:DI (match_operand:DI 0 "nonimmediate_operand" "+rm") (const_int 1) - (match_operand 1 "const_0_to_63_operand")) + (match_operand:QI 1 "const_0_to_63_operand")) (const_int 1)) (clobber (reg:CC FLAGS_REG))] "TARGET_64BIT && (TARGET_USE_BT || reload_completed)" @@ -16029,7 +16029,7 @@ (define_insn "*btrq_imm" [(set (zero_extract:DI (match_operand:DI 0 "nonimmediate_operand" "+rm") (const_int 1) - (match_operand 1 "const_0_to_63_operand")) + (match_operand:QI 1 "const_0_to_63_operand")) (const_int 0)) (clobber (reg:CC FLAGS_REG))] "TARGET_64BIT && (TARGET_USE_BT || reload_completed)" @@ -16042,7 +16042,7 @@ (define_insn "*btcq_imm" [(set (zero_extract:DI (match_operand:DI 0 "nonimmediate_operand" "+rm") (const_int 1) - (match_operand 1 "const_0_to_63_operand")) + (match_operand:QI 1 "const_0_to_63_operand")) (not:DI (zero_extract:DI (match_dup 0) (const_int 1) (match_dup 1)))) (clobber (reg:CC FLAGS_REG))] "TARGET_64BIT && (TARGET_USE_BT || reload_completed)" @@ -16059,7 +16059,7 @@ (parallel [(set (zero_extract:DI (match_operand:DI 0 "nonimmediate_operand") (const_int 1) - (match_operand 1 "const_0_to_63_operand")) + (match_operand:QI 1 "const_0_to_63_operand")) (const_int 1)) (clobber (reg:CC FLAGS_REG))])] "TARGET_64BIT && !TARGET_USE_BT" @@ -16083,7 +16083,7 @@ (parallel [(set (zero_extract:DI (match_operand:DI 0 "nonimmediate_operand") (const_int 1) - (match_operand 1 "const_0_to_63_operand")) + (match_operand:QI 1 "const_0_to_63_operand")) (const_int 0)) (clobber (reg:CC FLAGS_REG))])] "TARGET_64BIT && !TARGET_USE_BT" @@ -16107,7 +16107,7 @@ (parallel [(set (zero_extract:DI (match_operand:DI 0 "nonimmediate_operand") (const_int 1) - (match_operand 1 "const_0_to_63_operand")) + (match_operand:QI 1 "const_0_to_63_operand")) (not:DI (zero_extract:DI (match_dup 0) (const_int 1) (match_dup 1)))) (clobber (reg:CC FLAGS_REG))])] @@ -16135,14 +16135,14 @@ (zero_extract:SWI48 (match_operand:SWI48 0 "nonimmediate_operand" "r,m") (const_int 1) - (match_operand:SI 1 "nonmemory_operand" "r<S>,<S>")) + (match_operand:QI 1 "nonmemory_operand" "q<S>,<S>")) (const_int 0)))] "" { switch (get_attr_mode (insn)) { case MODE_SI: - return "bt{l}\t{%1, %k0|%k0, %1}"; + return "bt{l}\t{%k1, %k0|%k0, %k1}"; case MODE_DI: return "bt{q}\t{%q1, %0|%0, %q1}"; @@ -16160,13 +16160,36 @@ (const_string "SI") (const_string "<MODE>")))]) +(define_insn_and_split "*bt<SWI48:mode>_mask" + [(set (reg:CCC FLAGS_REG) + (compare:CCC + (zero_extract:SWI48 + (match_operand:SWI48 0 "nonimmediate_operand" "r,m") + (const_int 1) + (subreg:QI + (and:SWI248 + (match_operand:SWI248 1 "register_operand") + (match_operand 2 "const_int_operand")) 0)) + (const_int 0)))] + "TARGET_USE_BT + && (INTVAL (operands[2]) & (GET_MODE_BITSIZE (<SWI48:MODE>mode)-1)) + == GET_MODE_BITSIZE (<SWI48:MODE>mode)-1 + && ix86_pre_reload_split ()" + "#" + "&& 1" + [(set (reg:CCC FLAGS_REG) + (compare:CCC + (zero_extract:SWI48 (match_dup 0) (const_int 1) (match_dup 1)) + (const_int 0)))] + "operands[1] = gen_lowpart (QImode, operands[1]);") + (define_insn_and_split "*jcc_bt<mode>" [(set (pc) (if_then_else (match_operator 0 "bt_comparison_operator" [(zero_extract:SWI48 (match_operand:SWI48 1 "nonimmediate_operand") (const_int 1) - (match_operand:SI 2 "nonmemory_operand")) + (match_operand:QI 2 "nonmemory_operand")) (const_int 0)]) (label_ref (match_operand 3)) (pc))) @@ -16196,39 +16219,6 @@ PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0]))); }) -(define_insn_and_split "*jcc_bt<mode>_1" - [(set (pc) - (if_then_else (match_operator 0 "bt_comparison_operator" - [(zero_extract:SWI48 - (match_operand:SWI48 1 "register_operand") - (const_int 1) - (zero_extend:SI - (match_operand:QI 2 "register_operand"))) - (const_int 0)]) - (label_ref (match_operand 3)) - (pc))) - (clobber (reg:CC FLAGS_REG))] - "(TARGET_USE_BT || optimize_function_for_size_p (cfun)) - && ix86_pre_reload_split ()" - "#" - "&& 1" - [(set (reg:CCC FLAGS_REG) - (compare:CCC - (zero_extract:SWI48 - (match_dup 1) - (const_int 1) - (match_dup 2)) - (const_int 0))) - (set (pc) - (if_then_else (match_op_dup 0 [(reg:CCC FLAGS_REG) (const_int 0)]) - (label_ref (match_dup 3)) - (pc)))] -{ - operands[2] = lowpart_subreg (SImode, operands[2], QImode); - operands[0] = shallow_copy_rtx (operands[0]); - PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0]))); -}) - ;; Avoid useless masking of bit offset operand. (define_insn_and_split "*jcc_bt<mode>_mask" [(set (pc) @@ -16236,8 +16226,8 @@ [(zero_extract:SWI48 (match_operand:SWI48 1 "register_operand") (const_int 1) - (and:SI - (match_operand:SI 2 "register_operand") + (and:QI + (match_operand:QI 2 "register_operand") (match_operand 3 "const_int_operand")))]) (label_ref (match_operand 4)) (pc))) @@ -16264,23 +16254,23 @@ PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0]))); }) -(define_insn_and_split "*jcc_bt<mode>_mask_1" +;; Avoid useless masking of bit offset operand. +(define_insn_and_split "*jcc_bt<SWI48:mode>_mask_1" [(set (pc) - (if_then_else (match_operator 0 "bt_comparison_operator" + (if_then_else (match_operator 0 "bt_comparison_operator" [(zero_extract:SWI48 (match_operand:SWI48 1 "register_operand") (const_int 1) - (zero_extend:SI - (subreg:QI - (and - (match_operand 2 "int248_register_operand") - (match_operand 3 "const_int_operand")) 0)))]) + (subreg:QI + (and:SWI248 + (match_operand:SWI248 2 "register_operand") + (match_operand 3 "const_int_operand")) 0))]) (label_ref (match_operand 4)) (pc))) (clobber (reg:CC FLAGS_REG))] "(TARGET_USE_BT || optimize_function_for_size_p (cfun)) - && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (<MODE>mode)-1)) - == GET_MODE_BITSIZE (<MODE>mode)-1 + && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (<SWI48:MODE>mode)-1)) + == GET_MODE_BITSIZE (<SWI48:MODE>mode)-1 && ix86_pre_reload_split ()" "#" "&& 1" @@ -16296,10 +16286,9 @@ (label_ref (match_dup 4)) (pc)))] { - operands[2] = force_reg (GET_MODE (operands[2]), operands[2]); - operands[2] = gen_lowpart (SImode, operands[2]); operands[0] = shallow_copy_rtx (operands[0]); PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0]))); + operands[2] = gen_lowpart (QImode, operands[2]); }) ;; Help combine recognize bt followed by cmov @@ -16310,7 +16299,7 @@ [(zero_extract:SWI48 (match_operand:SWI48 1 "register_operand") (const_int 1) - (zero_extend:SI (match_operand:QI 2 "register_operand"))) + (match_operand:QI 2 "register_operand")) (const_int 0)]) (match_operand:SWI248 3 "nonimmediate_operand") (match_operand:SWI248 4 "nonimmediate_operand")))] @@ -16328,7 +16317,6 @@ { if (GET_CODE (operands[5]) == EQ) std::swap (operands[3], operands[4]); - operands[2] = lowpart_subreg (SImode, operands[2], QImode); }) ;; Help combine recognize bt followed by setc @@ -16337,7 +16325,7 @@ (zero_extract:SWI48 (match_operand:SWI48 1 "register_operand") (const_int 1) - (zero_extend:SI (match_operand:QI 2 "register_operand")))) + (match_operand:QI 2 "register_operand"))) (clobber (reg:CC FLAGS_REG))] "TARGET_USE_BT && ix86_pre_reload_split ()" "#" @@ -16347,8 +16335,7 @@ (zero_extract:SWI48 (match_dup 1) (const_int 1) (match_dup 2)) (const_int 0))) (set (match_dup 0) - (eq:QI (reg:CCC FLAGS_REG) (const_int 0)))] - "operands[2] = lowpart_subreg (SImode, operands[2], QImode);") + (eq:QI (reg:CCC FLAGS_REG) (const_int 0)))]) ;; Help combine recognize bt followed by setnc (define_insn_and_split "*bt<mode>_setncqi" @@ -16368,8 +16355,7 @@ (zero_extract:SWI48 (match_dup 1) (const_int 1) (match_dup 2)) (const_int 0))) (set (match_dup 0) - (ne:QI (reg:CCC FLAGS_REG) (const_int 0)))] - "operands[2] = lowpart_subreg (SImode, operands[2], QImode);") + (ne:QI (reg:CCC FLAGS_REG) (const_int 0)))]) (define_insn_and_split "*bt<mode>_setnc<mode>" [(set (match_operand:SWI48 0 "register_operand") @@ -16389,10 +16375,7 @@ (set (match_dup 3) (ne:QI (reg:CCC FLAGS_REG) (const_int 0))) (set (match_dup 0) (zero_extend:SWI48 (match_dup 3)))] -{ - operands[2] = lowpart_subreg (SImode, operands[2], QImode); - operands[3] = gen_reg_rtx (QImode); -}) + "operands[3] = gen_reg_rtx (QImode);") ;; Help combine recognize bt followed by setnc (PR target/110588) (define_insn_and_split "*bt<mode>_setncqi_2" @@ -16401,7 +16384,7 @@ (zero_extract:SWI48 (match_operand:SWI48 1 "register_operand") (const_int 1) - (zero_extend:SI (match_operand:QI 2 "register_operand"))) + (match_operand:QI 2 "register_operand")) (const_int 0))) (clobber (reg:CC FLAGS_REG))] "TARGET_USE_BT && ix86_pre_reload_split ()" @@ -16412,8 +16395,36 @@ (zero_extract:SWI48 (match_dup 1) (const_int 1) (match_dup 2)) (const_int 0))) (set (match_dup 0) - (ne:QI (reg:CCC FLAGS_REG) (const_int 0)))] - "operands[2] = lowpart_subreg (SImode, operands[2], QImode);") + (ne:QI (reg:CCC FLAGS_REG) (const_int 0)))]) + +;; Help combine recognize bt followed by setc +(define_insn_and_split "*bt<mode>_setc<mode>_mask" + [(set (match_operand:SWI48 0 "register_operand") + (zero_extract:SWI48 + (match_operand:SWI48 1 "register_operand") + (const_int 1) + (subreg:QI + (and:SWI48 + (match_operand:SWI48 2 "register_operand") + (match_operand 3 "const_int_operand")) 0))) + (clobber (reg:CC FLAGS_REG))] + "TARGET_USE_BT + && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (<MODE>mode)-1)) + == GET_MODE_BITSIZE (<MODE>mode)-1 + && ix86_pre_reload_split ()" + "#" + "&& 1" + [(set (reg:CCC FLAGS_REG) + (compare:CCC + (zero_extract:SWI48 (match_dup 1) (const_int 1) (match_dup 2)) + (const_int 0))) + (set (match_dup 3) + (eq:QI (reg:CCC FLAGS_REG) (const_int 0))) + (set (match_dup 0) (zero_extend:SWI48 (match_dup 3)))] +{ + operands[2] = gen_lowpart (QImode, operands[2]); + operands[3] = gen_reg_rtx (QImode); +}) ;; Store-flag instructions. @@ -18708,46 +18719,29 @@ [(parallel [(set (match_operand:SWI48 0 "register_operand") (if_then_else:SWI48 - (ne:QI (and:SWI48 (match_operand:SWI48 2 "register_operand") - (const_int 255)) + (ne:QI (match_operand:QI 2 "register_operand") (const_int 0)) (zero_extract:SWI48 (match_operand:SWI48 1 "nonimmediate_operand") - (umin:SWI48 (and:SWI48 (match_dup 2) (const_int 255)) - (match_dup 3)) + (umin:QI (match_dup 2) (match_dup 3)) (const_int 0)) (const_int 0))) (clobber (reg:CC FLAGS_REG))])] "TARGET_BMI2" - "operands[3] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT);") +{ + operands[2] = gen_lowpart (QImode, operands[2]); + operands[3] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); +}) (define_insn "*bmi2_bzhi_<mode>3" [(set (match_operand:SWI48 0 "register_operand" "=r") (if_then_else:SWI48 - (ne:QI (and:SWI48 (match_operand:SWI48 2 "register_operand" "r") - (const_int 255)) + (ne:QI (match_operand:QI 2 "register_operand" "q") (const_int 0)) (zero_extract:SWI48 (match_operand:SWI48 1 "nonimmediate_operand" "rm") - (umin:SWI48 (and:SWI48 (match_dup 2) (const_int 255)) - (match_operand:SWI48 3 "const_int_operand")) - (const_int 0)) - (const_int 0))) - (clobber (reg:CC FLAGS_REG))] - "TARGET_BMI2 && INTVAL (operands[3]) == <MODE_SIZE> * BITS_PER_UNIT" - "bzhi\t{%2, %1, %0|%0, %1, %2}" - [(set_attr "type" "bitmanip") - (set_attr "prefix" "vex") - (set_attr "mode" "<MODE>")]) - -(define_insn "*bmi2_bzhi_<mode>3_1" - [(set (match_operand:SWI48 0 "register_operand" "=r") - (if_then_else:SWI48 - (ne:QI (match_operand:QI 2 "register_operand" "r") (const_int 0)) - (zero_extract:SWI48 - (match_operand:SWI48 1 "nonimmediate_operand" "rm") - (umin:SWI48 (zero_extend:SWI48 (match_dup 2)) - (match_operand:SWI48 3 "const_int_operand")) + (umin:QI (match_dup 2) + (match_operand:QI 3 "const_int_operand")) (const_int 0)) (const_int 0))) (clobber (reg:CC FLAGS_REG))] @@ -18764,8 +18758,8 @@ (ne:QI (match_operand:QI 2 "register_operand" "r") (const_int 0)) (zero_extract:SWI48 (match_operand:SWI48 1 "nonimmediate_operand" "rm") - (umin:SWI48 (zero_extend:SWI48 (match_dup 2)) - (match_operand:SWI48 3 "const_int_operand")) + (umin:QI (match_dup 2) + (match_operand:QI 3 "const_int_operand")) (const_int 0)) (const_int 0)) (const_int 0))) @@ -18864,8 +18858,8 @@ [(set (match_operand:SWI48 0 "register_operand" "=r") (zero_extract:SWI48 (match_operand:SWI48 1 "nonimmediate_operand" "rm") - (match_operand 2 "const_0_to_255_operand") - (match_operand 3 "const_0_to_255_operand"))) + (match_operand:QI 2 "const_0_to_255_operand") + (match_operand:QI 3 "const_0_to_255_operand"))) (clobber (reg:CC FLAGS_REG))] "TARGET_TBM" { diff --git a/gcc/testsuite/gcc.target/i386/pr110790-1.c b/gcc/testsuite/gcc.target/i386/pr110790-1.c new file mode 100644 index 0000000..71733c3 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr110790-1.c @@ -0,0 +1,33 @@ +/* { dg-do run } */ +/* { dg-options "-O2" } */ +typedef unsigned long int mp_limb_t; +typedef const mp_limb_t * mp_srcptr; + +__attribute__((noipa)) +int +refmpn_tstbit_bad (mp_srcptr ptr, unsigned long bit) +{ + return (((ptr)[(bit)/(32 - 0)] & (((mp_limb_t) 1L) << ((bit)%(32 - 0)))) != 0); +} + +__attribute__((noipa, optimize(0))) +int +refmpn_tstbit_good (mp_srcptr ptr, unsigned long bit) +{ + return (((ptr)[(bit)/(32 - 0)] & (((mp_limb_t) 1L) << ((bit)%(32 - 0)))) != 0); +} + +__attribute__((noipa)) +int +refmpn_tstbit (mp_srcptr ptr, unsigned long bit) +{ + if (refmpn_tstbit_bad (ptr, bit) != refmpn_tstbit_good (ptr, bit)) { + __builtin_trap(); + } + return refmpn_tstbit_bad (ptr, bit); +} + +int main(){ + unsigned long num[] = { 0x3801ff9f, 0x0, 0x0, 0x0 }; + refmpn_tstbit(num, 0); +} diff --git a/gcc/testsuite/gcc.target/i386/pr110790-2.c b/gcc/testsuite/gcc.target/i386/pr110790-2.c new file mode 100644 index 0000000..8b9d650 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr110790-2.c @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ +typedef unsigned long int mp_limb_t; +typedef const mp_limb_t * mp_srcptr; + +int +refmpn_tstbit_bad (mp_srcptr ptr, unsigned long bit) +{ + return (((ptr)[(bit)/(32 - 0)] & (((mp_limb_t) 1L) << ((bit)%(32 - 0)))) != 0); +} + +/* { dg-final { scan-assembler "bt\[ql\]" } } */ +/* { dg-final { scan-assembler "setc" } } */