On Wed, 14 Nov 2018, Jakub Jelinek wrote:
> 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 where do the zero_extracts come from? Can we somehow avoid
zero-sized bit-extracts at expansion time by folding them to zero?
> 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?
I don't think those should be well-defined. If we would decide to
nail them down then both should yield zero indeed.
> 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
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
