On 06/24/2013 06:23 PM, Iyer, Balaji V wrote:
Actually, to reduce the amount of changes to non-AN code, let's put the AN case
third, after the offset and {} cases, so you get something like
else if (flag_enable_cilkplus)
{
tree an = cp_parser_array_notation (loc, parser, &index,
postfix_expression);
if (an)
return an;
/* Otherwise, cp_parser_array_notation set 'index'. */
}
else
index = cp_parser_expression (parser, /*cast_p=*/false, NULL);
this way the change is just a few added lines, and everything else is in
cp_parser_array_notation.
If I do it this way, then I don't think I will be able to handle a normal array
reference when cilk plus is enabled.
What I had in mind is that in the case of a normal array reference,
cp_parser_array_notation will update index (which is passed by address)
and return NULL_TREE, so that it gets back on the normal path.
One thing I could do is to assume that people won't use array notations in
braced list. I had it like this a while back but I made the change to make sure
I have covered more cases. Please let me know if that is OK and I will fix it.
Yes, that's OK.
I looked into this. But, magic_varargs_p is static to call.c. Should I make it
non-static?
Yes.
After making it non-static I was planning to do something like this:
if (magic_varargs_p (fndecl)
Nargs = (*params)->length ();
else
nargs = convert_arguments (...)
Is that OK with you?
I was thinking to check magic_varargs_p in convert_arguments, where it
currently has
if (fndecl && DECL_BUILT_IN (fndecl)
&& DECL_FUNCTION_CODE (fndecl) == BUILT_IN_CONSTANT_P)
/* Don't do ellipsis conversion for __built_in_constant_p
as this will result in spurious errors for non-trivial
types. */
change to "if (fndecl && magic_varargs_p (fndecl))"
By the way, why are you breaking out the elements of the
ARRAY_NOTATION_REF into a cilkplus_an_parts rather than using the _REF
directly?
I use the different parts of array notations for error checking and to create
the outer for-loop. Also, to create the ARRAY_REF I need the induction variable.
I understand the need for cilkplus_an_loop_parts, but cilkplus_an_parts
seems to contain exactly the same information as the ARRAY_NOTATION_REF.
Fixed! By the way, what is SFINAE?
SFINAE stands for "substitution failure is not an error". During
template argument deduction, once we have a full set of template
arguments we try to substitute them into the template declaration. In
that context, things that would normally cause an error just fail and
return error_mark_node silently.
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 55ed6a5..9d570b2 100644
Binary files a/gcc/cp/ChangeLog and b/gcc/cp/ChangeLog differ
This patch still has ChangeLog entries.
+ new_var = get_temp_regvar (TREE_TYPE (retval_expr),
+ build_zero_cst (TREE_TYPE (retval_expr)));
new_mod = expand_an_in_modify_expr (loc, new_var, NOP_EXPR,
+ TREE_OPERAND (retval_expr, 1),
+ tf_warning_or_error);
Why not pass TREE_OPERAND (retval_expr, 1) as the init to get_temp_regvar?
- parser->colon_corrects_to_scope_p = saved_colon_corrects_to_scope_p;
if (!length_index || length_index == error_mark_node)
cp_parser_skip_to_end_of_statement (parser);
@@ -6180,7 +6158,6 @@ cp_parser_array_notation (location_t loc, cp_parser
*parser, tree init_index,
saved_colon_corrects_to_scope_p =
parser->colon_corrects_to_scope_p;
/* Disable correcting single colon correcting to scope. */
- parser->colon_corrects_to_scope_p = false;
This change looks like now you will only restore
parser->colon_corrects_to_scope_p if you have a stride. I would suggest
putting back the first restore, and removing all the corrects_to_scope_p
code from the stride block, since there can't be an array-notation colon
after the stride.
+ /* If stride and start are of same type and the induction var
+ is not, convert induction variable to stride's type. */
+ if (TREE_TYPE (start) == TREE_TYPE (stride)
+ && TREE_TYPE (stride) != TREE_TYPE (var))
This seems impossible, since 'var' is created to have the same type as 'start'.
As you suggested further in the email, I have made var of type ptrdiff_type, so
I think I should keep this.
I think it would be better to convert start/stride to ptrdiff_t.
Incidentally, types should be compared with same_type_p rather than ==.
+ if (lhs_list_size > 0 && rhs_list_size > 0 && lhs_rank > 0 && rhs_rank > 0
+ && TREE_CODE (lhs_len) == INTEGER_CST && rhs_len
+ && TREE_CODE (rhs_len) == INTEGER_CST)
+ {
+ HOST_WIDE_INT l_length = int_cst_value (lhs_len);
+ HOST_WIDE_INT r_length = int_cst_value (rhs_len);
+ if (absu_hwi (l_length) != absu_hwi (r_length))
+ {
+ error_at (location, "length mismatch between LHS and RHS");
+ pop_stmt_list (an_init);
+ return error_mark_node;
+ }
+ }
Another place that should use tree_int_cst_equal.
+ /* No need to compare ii < rhs_rank && ii >= lhs_rank because valid Array
+ notation expression cannot RHS's rank cannot be greater than LHS. */
Too many "cannot"s.
+ if (processing_template_decl)
+ {
+ array_type = TREE_TYPE (array_value);
+ type = TREE_TYPE (array_type);
+ }
We should be able to parse array notation in a template even when the
array expression has unknown type. In a template, just parse and
remember the raw expressions without worrying about diagnostics and
conversions.
+ if (flag_enable_cilkplus && contains_array_notation_expr (expr))
+ {
+ size_t rank = 0;
+
+ if (!find_rank (input_location, expr, expr, false, &rank))
+ return error_mark_node;
+
+ /* If the return expression contains array notations, then flag it as
+ error. */
+ if (rank >= 1)
+ {
+ error_at (input_location, "array notation expression cannot be "
+ "used as a return value");
+ return error_mark_node;
+ }
+ }
...
+ /* If an array's index is an array notation, then its rank cannot be
+ greater than one. */
+ if (flag_enable_cilkplus && contains_array_notation_expr (idx))
+ {
+ size_t rank = 0;
+
+ /* If find_rank returns false, then it should have reported an error,
+ thus it is unnecessary for repetition. */
+ if (!find_rank (loc, idx, idx, true, &rank))
+ return error_mark_node;
+ if (rank > 1)
+ {
+ error_at (loc, "rank of the array%'s index is greater than 1");
+ return error_mark_node;
+ }
+ }
More diagnostics that should move to expansion time.
+ if (flag_enable_cilkplus && contains_array_notation_expr (op0))
+ type0 = find_correct_array_notation_type (op0);
+ else
+ type0 = TREE_TYPE (op0);
...
+ if (flag_enable_cilkplus && TREE_CODE (arg) == ARRAY_NOTATION_REF)
+ {
+ val = build_address (arg);
+ if (TREE_CODE (arg) == OFFSET_REF)
+ PTRMEM_OK_P (val) = PTRMEM_OK_P (arg);
+ return val;
+ }
...
+ /* If we are dealing with built-in array notation function then we don't need
+ to convert them. They will be broken up into modify exprs in future,
+ during which all these checks will be done. */
+ if (flag_enable_cilkplus
+ && is_cilkplus_reduce_builtin (fndecl) != BUILT_IN_NONE)
+ return rhs;
More changes that should be unnecessary since ARRAY_NOTATION_REF has a
normal type.
What remaining obstacles are there to sharing most of the expansion code
between C and C++? That can be a separate patch, of course.
Jason
