Re: [x86 PATCH] Fix issue with x86_64_const_vector_operand predicate.
On Sat, Jul 16, 2022 at 2:06 PM Roger Sayle 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 > > 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 > -- >
[x86 PATCH] Fix issue with x86_64_const_vector_operand predicate.
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 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 -- 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);