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

Reply via email to