| Message ID | 20240209092426.3E3B1139E7@imap1.dmz-prg2.suse.org |
|---|---|
| State | New |
| Headers | show |
| Series | middle-end/113576 - zero padding of vector bools when expanding compares | expand |
On Fri, Feb 09, 2024 at 10:24:25AM +0100, Richard Biener wrote: > The following zeros paddings of vector bools when expanding compares > and the mode used for the compare is an integer mode. In that case > targets cannot distinguish between a 4 element and 8 element vector > compare (both get to the QImode compare optab) so we have to do the > job in the middle-end. > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > OK? > > Thanks, > Richard. > > PR middle-end/113576 > * expr.cc (do_store_flag): For vector bool compares of vectors > with padding zero that. > * dojump.cc (do_compare_and_jump): Likewise. LGTM, but please give Richard time to chime in. Jakub
Richard Biener <rguenther@suse.de> writes: > The following zeros paddings of vector bools when expanding compares > and the mode used for the compare is an integer mode. In that case > targets cannot distinguish between a 4 element and 8 element vector > compare (both get to the QImode compare optab) so we have to do the > job in the middle-end. > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > OK? > > Thanks, > Richard. > > PR middle-end/113576 > * expr.cc (do_store_flag): For vector bool compares of vectors > with padding zero that. > * dojump.cc (do_compare_and_jump): Likewise. > --- > gcc/dojump.cc | 16 ++++++++++++++++ > gcc/expr.cc | 17 +++++++++++++++++ > 2 files changed, 33 insertions(+) > > diff --git a/gcc/dojump.cc b/gcc/dojump.cc > index e2d2b3cb111..ec2a365e488 100644 > --- a/gcc/dojump.cc > +++ b/gcc/dojump.cc > @@ -1266,6 +1266,7 @@ do_compare_and_jump (tree treeop0, tree treeop1, enum rtx_code signed_code, > machine_mode mode; > int unsignedp; > enum rtx_code code; > + unsigned HOST_WIDE_INT nunits; > > /* Don't crash if the comparison was erroneous. */ > op0 = expand_normal (treeop0); > @@ -1308,6 +1309,21 @@ do_compare_and_jump (tree treeop0, tree treeop1, enum rtx_code signed_code, > emit_insn (targetm.gen_canonicalize_funcptr_for_compare (new_op1, op1)); > op1 = new_op1; > } > + /* For boolean vectors with less than mode precision precision Too many precisions. LGTM otherwise, but could we put this in a shared helper, rather than duplicating the code? I'd be surprised if these are the only places we need to do something. Thanks, and sorry for the slow response (here and elsewhere). Richard > + make sure to fill padding with consistent values. */ > + else if (VECTOR_BOOLEAN_TYPE_P (type) > + && SCALAR_INT_MODE_P (mode) > + && TYPE_VECTOR_SUBPARTS (type).is_constant (&nunits) > + && maybe_ne (GET_MODE_PRECISION (mode), nunits)) > + { > + gcc_assert (code == EQ || code == NE); > + op0 = expand_binop (mode, and_optab, op0, > + GEN_INT ((1 << nunits) - 1), NULL_RTX, > + true, OPTAB_WIDEN); > + op1 = expand_binop (mode, and_optab, op1, > + GEN_INT ((1 << nunits) - 1), NULL_RTX, > + true, OPTAB_WIDEN); > + } > > do_compare_rtx_and_jump (op0, op1, code, unsignedp, treeop0, mode, > ((mode == BLKmode) > diff --git a/gcc/expr.cc b/gcc/expr.cc > index fc5e998e329..096081fdc53 100644 > --- a/gcc/expr.cc > +++ b/gcc/expr.cc > @@ -13502,6 +13502,7 @@ do_store_flag (sepops ops, rtx target, machine_mode mode) > rtx op0, op1; > rtx subtarget = target; > location_t loc = ops->location; > + unsigned HOST_WIDE_INT nunits; > > arg0 = ops->op0; > arg1 = ops->op1; > @@ -13694,6 +13695,22 @@ do_store_flag (sepops ops, rtx target, machine_mode mode) > > expand_operands (arg0, arg1, subtarget, &op0, &op1, EXPAND_NORMAL); > > + /* For boolean vectors with less than mode precision precision > + make sure to fill padding with consistent values. */ > + if (VECTOR_BOOLEAN_TYPE_P (type) > + && SCALAR_INT_MODE_P (operand_mode) > + && TYPE_VECTOR_SUBPARTS (type).is_constant (&nunits) > + && maybe_ne (GET_MODE_PRECISION (operand_mode), nunits)) > + { > + gcc_assert (code == EQ || code == NE); > + op0 = expand_binop (mode, and_optab, op0, > + GEN_INT ((1 << nunits) - 1), NULL_RTX, > + true, OPTAB_WIDEN); > + op1 = expand_binop (mode, and_optab, op1, > + GEN_INT ((1 << nunits) - 1), NULL_RTX, > + true, OPTAB_WIDEN); > + } > + > if (target == 0) > target = gen_reg_rtx (mode);
On Wed, 14 Feb 2024, Richard Sandiford wrote: > Richard Biener <rguenther@suse.de> writes: > > The following zeros paddings of vector bools when expanding compares > > and the mode used for the compare is an integer mode. In that case > > targets cannot distinguish between a 4 element and 8 element vector > > compare (both get to the QImode compare optab) so we have to do the > > job in the middle-end. > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > > > OK? > > > > Thanks, > > Richard. > > > > PR middle-end/113576 > > * expr.cc (do_store_flag): For vector bool compares of vectors > > with padding zero that. > > * dojump.cc (do_compare_and_jump): Likewise. > > --- > > gcc/dojump.cc | 16 ++++++++++++++++ > > gcc/expr.cc | 17 +++++++++++++++++ > > 2 files changed, 33 insertions(+) > > > > diff --git a/gcc/dojump.cc b/gcc/dojump.cc > > index e2d2b3cb111..ec2a365e488 100644 > > --- a/gcc/dojump.cc > > +++ b/gcc/dojump.cc > > @@ -1266,6 +1266,7 @@ do_compare_and_jump (tree treeop0, tree treeop1, enum rtx_code signed_code, > > machine_mode mode; > > int unsignedp; > > enum rtx_code code; > > + unsigned HOST_WIDE_INT nunits; > > > > /* Don't crash if the comparison was erroneous. */ > > op0 = expand_normal (treeop0); > > @@ -1308,6 +1309,21 @@ do_compare_and_jump (tree treeop0, tree treeop1, enum rtx_code signed_code, > > emit_insn (targetm.gen_canonicalize_funcptr_for_compare (new_op1, op1)); > > op1 = new_op1; > > } > > + /* For boolean vectors with less than mode precision precision > > Too many precisions. Fixed. > LGTM otherwise, but could we put this in a shared helper, rather than > duplicating the code? I'd be surprised if these are the only places > we need to do something. Let's think of this when we get to more places. I guess you are thinking of the if condition here, right? Pushed with the comment fix for now. Thanks, Richard. > Thanks, and sorry for the slow response (here and elsewhere). > > Richard > > > + make sure to fill padding with consistent values. */ > > + else if (VECTOR_BOOLEAN_TYPE_P (type) > > + && SCALAR_INT_MODE_P (mode) > > + && TYPE_VECTOR_SUBPARTS (type).is_constant (&nunits) > > + && maybe_ne (GET_MODE_PRECISION (mode), nunits)) > > + { > > + gcc_assert (code == EQ || code == NE); > > + op0 = expand_binop (mode, and_optab, op0, > > + GEN_INT ((1 << nunits) - 1), NULL_RTX, > > + true, OPTAB_WIDEN); > > + op1 = expand_binop (mode, and_optab, op1, > > + GEN_INT ((1 << nunits) - 1), NULL_RTX, > > + true, OPTAB_WIDEN); > > + } > > > > do_compare_rtx_and_jump (op0, op1, code, unsignedp, treeop0, mode, > > ((mode == BLKmode) > > diff --git a/gcc/expr.cc b/gcc/expr.cc > > index fc5e998e329..096081fdc53 100644 > > --- a/gcc/expr.cc > > +++ b/gcc/expr.cc > > @@ -13502,6 +13502,7 @@ do_store_flag (sepops ops, rtx target, machine_mode mode) > > rtx op0, op1; > > rtx subtarget = target; > > location_t loc = ops->location; > > + unsigned HOST_WIDE_INT nunits; > > > > arg0 = ops->op0; > > arg1 = ops->op1; > > @@ -13694,6 +13695,22 @@ do_store_flag (sepops ops, rtx target, machine_mode mode) > > > > expand_operands (arg0, arg1, subtarget, &op0, &op1, EXPAND_NORMAL); > > > > + /* For boolean vectors with less than mode precision precision > > + make sure to fill padding with consistent values. */ > > + if (VECTOR_BOOLEAN_TYPE_P (type) > > + && SCALAR_INT_MODE_P (operand_mode) > > + && TYPE_VECTOR_SUBPARTS (type).is_constant (&nunits) > > + && maybe_ne (GET_MODE_PRECISION (operand_mode), nunits)) > > + { > > + gcc_assert (code == EQ || code == NE); > > + op0 = expand_binop (mode, and_optab, op0, > > + GEN_INT ((1 << nunits) - 1), NULL_RTX, > > + true, OPTAB_WIDEN); > > + op1 = expand_binop (mode, and_optab, op1, > > + GEN_INT ((1 << nunits) - 1), NULL_RTX, > > + true, OPTAB_WIDEN); > > + } > > + > > if (target == 0) > > target = gen_reg_rtx (mode); >
diff --git a/gcc/dojump.cc b/gcc/dojump.cc index e2d2b3cb111..ec2a365e488 100644 --- a/gcc/dojump.cc +++ b/gcc/dojump.cc @@ -1266,6 +1266,7 @@ do_compare_and_jump (tree treeop0, tree treeop1, enum rtx_code signed_code, machine_mode mode; int unsignedp; enum rtx_code code; + unsigned HOST_WIDE_INT nunits; /* Don't crash if the comparison was erroneous. */ op0 = expand_normal (treeop0); @@ -1308,6 +1309,21 @@ do_compare_and_jump (tree treeop0, tree treeop1, enum rtx_code signed_code, emit_insn (targetm.gen_canonicalize_funcptr_for_compare (new_op1, op1)); op1 = new_op1; } + /* For boolean vectors with less than mode precision precision + make sure to fill padding with consistent values. */ + else if (VECTOR_BOOLEAN_TYPE_P (type) + && SCALAR_INT_MODE_P (mode) + && TYPE_VECTOR_SUBPARTS (type).is_constant (&nunits) + && maybe_ne (GET_MODE_PRECISION (mode), nunits)) + { + gcc_assert (code == EQ || code == NE); + op0 = expand_binop (mode, and_optab, op0, + GEN_INT ((1 << nunits) - 1), NULL_RTX, + true, OPTAB_WIDEN); + op1 = expand_binop (mode, and_optab, op1, + GEN_INT ((1 << nunits) - 1), NULL_RTX, + true, OPTAB_WIDEN); + } do_compare_rtx_and_jump (op0, op1, code, unsignedp, treeop0, mode, ((mode == BLKmode) diff --git a/gcc/expr.cc b/gcc/expr.cc index fc5e998e329..096081fdc53 100644 --- a/gcc/expr.cc +++ b/gcc/expr.cc @@ -13502,6 +13502,7 @@ do_store_flag (sepops ops, rtx target, machine_mode mode) rtx op0, op1; rtx subtarget = target; location_t loc = ops->location; + unsigned HOST_WIDE_INT nunits; arg0 = ops->op0; arg1 = ops->op1; @@ -13694,6 +13695,22 @@ do_store_flag (sepops ops, rtx target, machine_mode mode) expand_operands (arg0, arg1, subtarget, &op0, &op1, EXPAND_NORMAL); + /* For boolean vectors with less than mode precision precision + make sure to fill padding with consistent values. */ + if (VECTOR_BOOLEAN_TYPE_P (type) + && SCALAR_INT_MODE_P (operand_mode) + && TYPE_VECTOR_SUBPARTS (type).is_constant (&nunits) + && maybe_ne (GET_MODE_PRECISION (operand_mode), nunits)) + { + gcc_assert (code == EQ || code == NE); + op0 = expand_binop (mode, and_optab, op0, + GEN_INT ((1 << nunits) - 1), NULL_RTX, + true, OPTAB_WIDEN); + op1 = expand_binop (mode, and_optab, op1, + GEN_INT ((1 << nunits) - 1), NULL_RTX, + true, OPTAB_WIDEN); + } + if (target == 0) target = gen_reg_rtx (mode);