Hi! As mentioned in the PR, it is unclear if zero_extract is well defined if the second argument is 0. x86 intrinsic require bzhi and bextr to be well defined in those cases (extraction of 0 bits results in 0), but e.g. combiner hapilly transforms that into << 64 >> 64, simplify-rtx.c, while it folds it into 0, might invoke UB in the compiler etc.
So, is (zero_extract x 0 y) well defined in the middle-end or not? If it is well defined, then what about (sign_extract x 0 y), does it also yield 0, undefined, something else? If neither of those are well defined, the following patch provides a backend workaround for that, by making sure the second argument of zero_extract is never 0 on x86. Bootstrapped/regtested on x86_64-linux and i686-linux. 2018-11-13 Jakub Jelinek <ja...@redhat.com> PR rtl-optimization/87817 * config/i386/i386.md (nmi2_bzhi_<mode>3, *bmi2_bzhi_<mode>3, *bmi2_bzhi_<mode>3_1, *bmi2_bzhi_<mode>3_1_ccz): Use IF_THEN_ELSE in the pattern to avoid triggering UB when operands[2] is zero. (tbm_bextri_<mode>): New expander. Renamed the old define_insn to ... (*tbm_bextri_<mode>): ... this. --- gcc/config/i386/i386.md.jj 2018-11-09 14:02:00.030267540 +0100 +++ gcc/config/i386/i386.md 2018-11-13 15:45:33.034870609 +0100 @@ -13614,12 +13614,15 @@ (define_insn "*bmi_blsr_<mode>_ccz" (define_expand "bmi2_bzhi_<mode>3" [(parallel [(set (match_operand:SWI48 0 "register_operand") - (zero_extract:SWI48 - (match_operand:SWI48 1 "nonimmediate_operand") - (umin:SWI48 - (and:SWI48 (match_operand:SWI48 2 "register_operand") - (const_int 255)) - (match_dup 3)) + (if_then_else:SWI48 + (ne:QI (and:SWI48 (match_operand:SWI48 2 "register_operand") + (const_int 255)) + (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)) + (const_int 0)) (const_int 0))) (clobber (reg:CC FLAGS_REG))])] "TARGET_BMI2" @@ -13627,12 +13630,15 @@ (define_expand "bmi2_bzhi_<mode>3" (define_insn "*bmi2_bzhi_<mode>3" [(set (match_operand:SWI48 0 "register_operand" "=r") - (zero_extract:SWI48 - (match_operand:SWI48 1 "nonimmediate_operand" "rm") - (umin:SWI48 - (and:SWI48 (match_operand:SWI48 2 "register_operand" "r") - (const_int 255)) - (match_operand:SWI48 3 "const_int_operand" "n")) + (if_then_else:SWI48 + (ne:QI (and:SWI48 (match_operand:SWI48 2 "register_operand" "r") + (const_int 255)) + (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" "n")) + (const_int 0)) (const_int 0))) (clobber (reg:CC FLAGS_REG))] "TARGET_BMI2 && INTVAL (operands[3]) == <MODE_SIZE> * BITS_PER_UNIT" @@ -13643,11 +13649,13 @@ (define_insn "*bmi2_bzhi_<mode>3" (define_insn "*bmi2_bzhi_<mode>3_1" [(set (match_operand:SWI48 0 "register_operand" "=r") - (zero_extract:SWI48 - (match_operand:SWI48 1 "nonimmediate_operand" "rm") - (umin:SWI48 - (zero_extend:SWI48 (match_operand:QI 2 "register_operand" "r")) - (match_operand:SWI48 3 "const_int_operand" "n")) + (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" "n")) + (const_int 0)) (const_int 0))) (clobber (reg:CC FLAGS_REG))] "TARGET_BMI2 && INTVAL (operands[3]) == <MODE_SIZE> * BITS_PER_UNIT" @@ -13659,11 +13667,13 @@ (define_insn "*bmi2_bzhi_<mode>3_1" (define_insn "*bmi2_bzhi_<mode>3_1_ccz" [(set (reg:CCZ FLAGS_REG) (compare:CCZ - (zero_extract:SWI48 - (match_operand:SWI48 1 "nonimmediate_operand" "rm") - (umin:SWI48 - (zero_extend:SWI48 (match_operand:QI 2 "register_operand" "r")) - (match_operand:SWI48 3 "const_int_operand" "n")) + (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" "n")) + (const_int 0)) (const_int 0)) (const_int 0))) (clobber (match_scratch:SWI48 0 "=r"))] @@ -13696,7 +13706,28 @@ (define_insn "bmi2_pext_<mode>3" (set_attr "mode" "<MODE>")]) ;; TBM instructions. -(define_insn "tbm_bextri_<mode>" +(define_expand "tbm_bextri_<mode>" + [(parallel + [(set (match_operand:SWI48 0 "register_operand") + (zero_extract:SWI48 + (match_operand:SWI48 1 "nonimmediate_operand") + (match_operand 2 "const_0_to_255_operand" "N") + (match_operand 3 "const_0_to_255_operand" "N"))) + (clobber (reg:CC FLAGS_REG))])] + "TARGET_TBM" +{ + if (operands[2] == const0_rtx + || INTVAL (operands[3]) >= <MODE_SIZE> * BITS_PER_UNIT) + { + emit_move_insn (operands[0], const0_rtx); + DONE; + } + if (INTVAL (operands[2]) + INTVAL (operands[3]) + > <MODE_SIZE> * BITS_PER_UNIT) + operands[2] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT - INTVAL (operands[3])); +}) + +(define_insn "*tbm_bextri_<mode>" [(set (match_operand:SWI48 0 "register_operand" "=r") (zero_extract:SWI48 (match_operand:SWI48 1 "nonimmediate_operand" "rm") Jakub