+/* Build a VEC_CONVERT ifn for __builtin_convertvector builtin. */
Can you please document the function arguments and explain how they are used?
+ +tree +c_build_vec_convert (location_t loc1, tree expr, location_t loc2, tree type, + bool complain) +{ + if (error_operand_p (type)) + return error_mark_node; + if (error_operand_p (expr)) + return error_mark_node; + + if (!VECTOR_INTEGER_TYPE_P (TREE_TYPE (expr)) + && !VECTOR_FLOAT_TYPE_P (TREE_TYPE (expr))) + { + if (complain) + error_at (loc1, "%<__builtin_convertvector%> first argument must " + "be an integer or floating vector"); + return error_mark_node; + } + + if (!VECTOR_INTEGER_TYPE_P (type) && !VECTOR_FLOAT_TYPE_P (type)) + { + if (complain) + error_at (loc2, "%<__builtin_convertvector%> second argument must " + "be an integer or floating vector type"); + return error_mark_node; + } + + if (maybe_ne (TYPE_VECTOR_SUBPARTS (TREE_TYPE (expr)), + TYPE_VECTOR_SUBPARTS (type))) + { + if (complain) + error_at (loc1, "%<__builtin_convertvector%> number of elements " + "of the first argument vector and the second argument " + "vector type should be the same"); + return error_mark_node; + }
Just a few wording suggestions for the errors: 1) for the first two errors consider using a single message parameterized on the argument number to reduce translation effort (both styles are in use but the more concise form seems preferable to me) 2) in the last error use "must" instead of "should" as in the first two ("must" is imperative rather than just suggestive) 3) consider simplifying the third message to "%<...%> argument vectors must have the same size" (or "the same number of elements") along the same lines as in c_build_vec_perm_expr.
+ + if ((TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (expr))) + == TYPE_MAIN_VARIANT (TREE_TYPE (type))) + || (VECTOR_INTEGER_TYPE_P (TREE_TYPE (expr)) + && VECTOR_INTEGER_TYPE_P (type) + && (TYPE_PRECISION (TREE_TYPE (TREE_TYPE (expr))) + == TYPE_PRECISION (TREE_TYPE (type))))) + return build1_loc (loc1, VIEW_CONVERT_EXPR, type, expr);
The conditional above is very difficult to read, and without a comment explaining its purpose, difficult to understand. Introducing named temporaries for the repetitive subexpressions would help with the readability (not just here but in rest of the function as well). A comment explaining what the conditional handles would help with the latter.
+ + bool wrap = true; + bool maybe_const = false; + tree ret;
Moving maybe_const to the conditional block where it's used and ret to the point of its initialization just after that block would improve readability.
+ if (!c_dialect_cxx ()) + { + /* Avoid C_MAYBE_CONST_EXPRs inside of VEC_CONVERT argument. */ + expr = c_fully_fold (expr, false, &maybe_const); + wrap &= maybe_const; + } + + ret = build_call_expr_internal_loc (loc1, IFN_VEC_CONVERT, type, 1, expr); + + if (!wrap) + ret = c_wrap_maybe_const (ret, true); + + return ret; }
Martin