| Message ID | alpine.DEB.2.02.1211012153290.31724@stedding.saclay.inria.fr |
|---|---|
| State | New |
| Headers | show |
On Thu, Nov 1, 2012 at 10:10 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > Hello, > > this patch makes gimple checking of vector comparisons more strict by > ensuring that it doesn't return a boolean. It also fixes a bug that this > check detected in fold-const.c: for (void)(x<0), the C front-end was calling > fold_unary_loc on the conversion to void, which was then converted to > if(x<0)(void)0. > > 2012-11-02 Marc Glisse <marc.glisse@inria.fr> > > * tree-cfg.c (verify_gimple_comparison): Verify that vector > comparison returns a vector. > * fold-const.c (fold_unary_loc): Disable conversion optimization > for void type. > > -- > Marc Glisse > Index: gcc/fold-const.c > =================================================================== > --- gcc/fold-const.c (revision 193060) > +++ gcc/fold-const.c (working copy) > @@ -7742,21 +7742,22 @@ fold_unary_loc (location_t loc, enum tre > /* If we have (type) (a CMP b) and type is an integral type, > return > new expression involving the new type. Canonicalize > (type) (a CMP b) to (a CMP b) ? (type) true : (type) false for > non-integral type. > Do not fold the result as that would not simplify further, also > folding again results in recursions. */ > if (TREE_CODE (type) == BOOLEAN_TYPE) > return build2_loc (loc, TREE_CODE (op0), type, > TREE_OPERAND (op0, 0), > TREE_OPERAND (op0, 1)); > - else if (!INTEGRAL_TYPE_P (type) && TREE_CODE (type) != > VECTOR_TYPE) > + else if (!INTEGRAL_TYPE_P (type) && !VOID_TYPE_P (type) > + && TREE_CODE (type) != VECTOR_TYPE) > return build3_loc (loc, COND_EXPR, type, op0, > constant_boolean_node (true, type), > constant_boolean_node (false, type)); > } > > /* Handle cases of two conversions in a row. */ > if (CONVERT_EXPR_P (op0)) > { > tree inside_type = TREE_TYPE (TREE_OPERAND (op0, 0)); > tree inter_type = TREE_TYPE (op0); Ok for this part. > Index: gcc/tree-cfg.c > =================================================================== > --- gcc/tree-cfg.c (revision 193060) > +++ gcc/tree-cfg.c (working copy) > @@ -3263,21 +3263,30 @@ verify_gimple_comparison (tree type, tre > error ("mismatching comparison operand types"); > debug_generic_expr (op0_type); > debug_generic_expr (op1_type); > return true; > } > > /* The resulting type of a comparison may be an effective boolean type. > */ > if (INTEGRAL_TYPE_P (type) > && (TREE_CODE (type) == BOOLEAN_TYPE > || TYPE_PRECISION (type) == 1)) > - ; > + { > + if (TREE_CODE (op0_type) == VECTOR_TYPE > + || TREE_CODE (op1_type) == VECTOR_TYPE) > + { > + error ("vector comparison returning a boolean"); > + debug_generic_expr (op0_type); > + debug_generic_expr (op1_type); > + return true; > + } verify_gimple_* should have "positive" checks, thus, check that if there are vector operands the comparison result should be a vector. Not complaining about a vector comparison having a boolean result. Richard. > + } > /* Or an integer vector type with the same size and element count > as the comparison operand types. */ > else if (TREE_CODE (type) == VECTOR_TYPE > && TREE_CODE (TREE_TYPE (type)) == INTEGER_TYPE) > { > if (TREE_CODE (op0_type) != VECTOR_TYPE > || TREE_CODE (op1_type) != VECTOR_TYPE) > { > error ("non-vector operands in vector comparison"); > debug_generic_expr (op0_type); >
(I forgot to send this at the time) On Sun, 4 Nov 2012, Richard Biener wrote: >> - else if (!INTEGRAL_TYPE_P (type) && TREE_CODE (type) != >> VECTOR_TYPE) >> + else if (!INTEGRAL_TYPE_P (type) && !VOID_TYPE_P (type) >> + && TREE_CODE (type) != VECTOR_TYPE) [...] > Ok for this part. Applied, thanks. >> Index: gcc/tree-cfg.c >> =================================================================== >> --- gcc/tree-cfg.c (revision 193060) >> +++ gcc/tree-cfg.c (working copy) >> @@ -3263,21 +3263,30 @@ verify_gimple_comparison (tree type, tre >> error ("mismatching comparison operand types"); >> debug_generic_expr (op0_type); >> debug_generic_expr (op1_type); >> return true; >> } >> >> /* The resulting type of a comparison may be an effective boolean type. >> */ >> if (INTEGRAL_TYPE_P (type) >> && (TREE_CODE (type) == BOOLEAN_TYPE >> || TYPE_PRECISION (type) == 1)) >> - ; >> + { >> + if (TREE_CODE (op0_type) == VECTOR_TYPE >> + || TREE_CODE (op1_type) == VECTOR_TYPE) >> + { >> + error ("vector comparison returning a boolean"); >> + debug_generic_expr (op0_type); >> + debug_generic_expr (op1_type); >> + return true; >> + } > > verify_gimple_* should have "positive" checks, thus, check that > if there are vector operands the comparison result should be a > vector. Not complaining about a vector comparison having a > boolean result. I wasn't sure what that was supposed to look like, so I dropped it for now.
On Thu, Nov 22, 2012 at 11:59 AM, Marc Glisse <marc.glisse@inria.fr> wrote: > (I forgot to send this at the time) > > > On Sun, 4 Nov 2012, Richard Biener wrote: > >>> - else if (!INTEGRAL_TYPE_P (type) && TREE_CODE (type) != >>> VECTOR_TYPE) >>> + else if (!INTEGRAL_TYPE_P (type) && !VOID_TYPE_P (type) >>> + && TREE_CODE (type) != VECTOR_TYPE) > > [...] >> >> Ok for this part. > > > Applied, thanks. > > >>> Index: gcc/tree-cfg.c >>> =================================================================== >>> --- gcc/tree-cfg.c (revision 193060) >>> +++ gcc/tree-cfg.c (working copy) >>> @@ -3263,21 +3263,30 @@ verify_gimple_comparison (tree type, tre >>> error ("mismatching comparison operand types"); >>> debug_generic_expr (op0_type); >>> debug_generic_expr (op1_type); >>> return true; >>> } >>> >>> /* The resulting type of a comparison may be an effective boolean >>> type. >>> */ >>> if (INTEGRAL_TYPE_P (type) >>> && (TREE_CODE (type) == BOOLEAN_TYPE >>> || TYPE_PRECISION (type) == 1)) >>> - ; >>> + { >>> + if (TREE_CODE (op0_type) == VECTOR_TYPE >>> + || TREE_CODE (op1_type) == VECTOR_TYPE) >>> + { >>> + error ("vector comparison returning a boolean"); >>> + debug_generic_expr (op0_type); >>> + debug_generic_expr (op1_type); >>> + return true; >>> + } >> >> >> verify_gimple_* should have "positive" checks, thus, check that >> if there are vector operands the comparison result should be a >> vector. Not complaining about a vector comparison having a >> boolean result. > > > I wasn't sure what that was supposed to look like, so I dropped it for now. Ok, looking closer we have /* The resulting type of a comparison may be an effective boolean type. */ if (INTEGRAL_TYPE_P (type) && (TREE_CODE (type) == BOOLEAN_TYPE || TYPE_PRECISION (type) == 1)) ; /* Or an integer vector type with the same size and element count as the comparison operand types. */ else if (TREE_CODE (type) == VECTOR_TYPE && TREE_CODE (TREE_TYPE (type)) == INTEGER_TYPE) { ... Thus you are rejecting a boolean valued vector comparison which we otherwise happily accept. I suppose that makes sense (even though at least equality compares can make sense). Thus that hunk is ok as well. Thanks, Richard. > -- > Marc Glisse
On Mon, 26 Nov 2012, Richard Biener wrote: > Thus you are rejecting a boolean valued vector comparison which we > otherwise happily accept. I suppose that makes sense (even though > at least equality compares can make sense). I agree that boolean equality comparison of vectors makes sense, but I don't think the code is ready to properly expand it. I guess we'll have to revisit this at some point. > Thus that hunk is ok as well. Done, thanks.
Index: gcc/fold-const.c =================================================================== --- gcc/fold-const.c (revision 193060) +++ gcc/fold-const.c (working copy) @@ -7742,21 +7742,22 @@ fold_unary_loc (location_t loc, enum tre /* If we have (type) (a CMP b) and type is an integral type, return new expression involving the new type. Canonicalize (type) (a CMP b) to (a CMP b) ? (type) true : (type) false for non-integral type. Do not fold the result as that would not simplify further, also folding again results in recursions. */ if (TREE_CODE (type) == BOOLEAN_TYPE) return build2_loc (loc, TREE_CODE (op0), type, TREE_OPERAND (op0, 0), TREE_OPERAND (op0, 1)); - else if (!INTEGRAL_TYPE_P (type) && TREE_CODE (type) != VECTOR_TYPE) + else if (!INTEGRAL_TYPE_P (type) && !VOID_TYPE_P (type) + && TREE_CODE (type) != VECTOR_TYPE) return build3_loc (loc, COND_EXPR, type, op0, constant_boolean_node (true, type), constant_boolean_node (false, type)); } /* Handle cases of two conversions in a row. */ if (CONVERT_EXPR_P (op0)) { tree inside_type = TREE_TYPE (TREE_OPERAND (op0, 0)); tree inter_type = TREE_TYPE (op0); Index: gcc/tree-cfg.c =================================================================== --- gcc/tree-cfg.c (revision 193060) +++ gcc/tree-cfg.c (working copy) @@ -3263,21 +3263,30 @@ verify_gimple_comparison (tree type, tre error ("mismatching comparison operand types"); debug_generic_expr (op0_type); debug_generic_expr (op1_type); return true; } /* The resulting type of a comparison may be an effective boolean type. */ if (INTEGRAL_TYPE_P (type) && (TREE_CODE (type) == BOOLEAN_TYPE || TYPE_PRECISION (type) == 1)) - ; + { + if (TREE_CODE (op0_type) == VECTOR_TYPE + || TREE_CODE (op1_type) == VECTOR_TYPE) + { + error ("vector comparison returning a boolean"); + debug_generic_expr (op0_type); + debug_generic_expr (op1_type); + return true; + } + } /* Or an integer vector type with the same size and element count as the comparison operand types. */ else if (TREE_CODE (type) == VECTOR_TYPE && TREE_CODE (TREE_TYPE (type)) == INTEGER_TYPE) { if (TREE_CODE (op0_type) != VECTOR_TYPE || TREE_CODE (op1_type) != VECTOR_TYPE) { error ("non-vector operands in vector comparison"); debug_generic_expr (op0_type);