diff mbox series

[x86] Fix issue with x86_64_const_vector_operand predicate.

Message ID 008201d8990c$8ce7ac00$a6b70400$@nextmovesoftware.com
State New
Headers show
Series [x86] Fix issue with x86_64_const_vector_operand predicate. | expand

Commit Message

Roger Sayle July 16, 2022, 12:06 p.m. UTC 
This patch fixes (what I believe is) a latent bug in i386.md's
x86_64_const_vector_operand define_predicate.  According to the
documentation, when a predicate is called with rtx operand OP and
machine_mode operand MODE, we can't shouldn't assume that the
MODE is (or has been checked to be) GET_MODE (OP).

The failure mode is that recog can call x86_64_const_vector_operand
on an arbitrary CONST_VECTOR passing a MODE of V2QI_mode, but when
the CONST_VECTOR is in fact V1TImode, it's unsafe to directly call
ix86_convert_const_vector_to_integer, which assumes that the CONST_VECTOR
contains CONST_INTs when it actually contains CONST_WIDE_INTs.  The
checks in this define_predicate need to be testing OP's mode, and
ideally confirming that this matches the passed in/specified MODE.

This bug is currently latent, but adding an innocent/unrelated
define_insn, such as "(set (reg:CCC FLAGS_REG) (const_int 0))" to
i386.md can occasionally change the order in which genrecog generates
its tests, then ICEing during bootstrap due to V1TI CONST_VECTORs.


This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
and make -k check, both with and without --target-board=unix{-m32},
with no new failures.  Ok for mainline?

2022-07-16  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
        * config/i386/predicates.md (x86_64_const_vector_operand):
        Check the operand's mode matches the specified mode argument.


Thanks in advance,
Roger
--

Comments

Uros Bizjak July 18, 2022, 6:07 a.m. UTC | #1 
On Sat, Jul 16, 2022 at 2:06 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> This patch fixes (what I believe is) a latent bug in i386.md's
> x86_64_const_vector_operand define_predicate.  According to the
> documentation, when a predicate is called with rtx operand OP and
> machine_mode operand MODE, we can't shouldn't assume that the
> MODE is (or has been checked to be) GET_MODE (OP).
>
> The failure mode is that recog can call x86_64_const_vector_operand
> on an arbitrary CONST_VECTOR passing a MODE of V2QI_mode, but when
> the CONST_VECTOR is in fact V1TImode, it's unsafe to directly call
> ix86_convert_const_vector_to_integer, which assumes that the CONST_VECTOR
> contains CONST_INTs when it actually contains CONST_WIDE_INTs.  The
> checks in this define_predicate need to be testing OP's mode, and
> ideally confirming that this matches the passed in/specified MODE.
>
> This bug is currently latent, but adding an innocent/unrelated
> define_insn, such as "(set (reg:CCC FLAGS_REG) (const_int 0))" to
> i386.md can occasionally change the order in which genrecog generates
> its tests, then ICEing during bootstrap due to V1TI CONST_VECTORs.
>
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and without --target-board=unix{-m32},
> with no new failures.  Ok for mainline?
>
> 2022-07-16  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
>         * config/i386/predicates.md (x86_64_const_vector_operand):
>         Check the operand's mode matches the specified mode argument.

OK.

Thanks,
Uros.

>
>
> Thanks in advance,
> Roger
> --
>
diff mbox series

Patch

diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index c71c453..42053ea 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -1199,6 +1199,10 @@ 
 (define_predicate "x86_64_const_vector_operand"
   (match_code "const_vector")
 {
+  if (mode == VOIDmode)
+    mode = GET_MODE (op);
+  else if (GET_MODE (op) != mode)
+    return false;
   if (GET_MODE_SIZE (mode) > UNITS_PER_WORD)
     return false;
   HOST_WIDE_INT val = ix86_convert_const_vector_to_integer (op, mode);