On 07/29/2015 06:56 PM, Kai Tietz wrote:
@@ -1430,6 +1438,8 @@ cxx_eval_call_expression (const constexpr_ctx
*ctx,
tree t,
bool
reduced_constant_expression_p (tree t)
{
+ /* Make sure we remove useless initial NOP_EXPRs. */
+ STRIP_NOPS (t);
Checked, and removing those STRIP_NOPS cause regressions about
vector-casts. At least the STRIP_NOPS in
reduced_constant_expression_p seems to be required. See as example
g++.dg/ext/vector20.C as testcase.
It sees that '(vec)(const __vector(2) long int){3l, 4l}' is not a
constant expression.
But when was that NOP_EXPR added? It should have been folded away
before we get here.
case SIZEOF_EXPR:
+ if (processing_template_decl
+ && (!COMPLETE_TYPE_P (TREE_TYPE (t))
+ || TREE_CODE (TYPE_SIZE (TREE_TYPE (t))) != INTEGER_CST))
+ return t;
Why is this necessary?
The issue is that by delayed-folding we don't fold sizeof-expressions
until we do the folding after genericize-pass. So those expressions
remain, and we can run in template on sizeof-operators on incomplete
types, if we invoke here variants of the constexpr-code. So this
pattern simply verifies that the sizeof-operand can be determined. We
could simply avoid resolving sizeof-operators in template-decl at all.
But my idea here was to try to resolve them, if the type of the
operand is already complete (and has an constant size).
But this condition will never be true, as TREE_TYPE (t) is always
size_t. So this code isn't actually addressing the situation you describe.
We don't want to resolve SIZEOF_EXPR within template-declarations for
incomplete types, of if its size isn't fixed. Issue is that we
otherwise get issues about expressions without existing type (as usual
within template-declarations for some expressions).
Yes, but we shouldn't have gotten this far with a dependent sizeof;
maybe_constant_value just returns if
instantiation_dependent_expression_p is true.
Well, but we could come here by other routine then
maybe_constant_value. For example cxx_constnat_value doesn't do checks
here.
Calling cxx_constant_value on a dependent expression will tend to ICE,
so we don't need to worry about that.
@@ -8496,16 +8467,18 @@ compute_array_index_type (tree name, tree
size,
tsubst_flags_t complain)
SET_TYPE_STRUCTURAL_EQUALITY (itype);
return itype;
}
-
+
+ /* We need to do fully folding to determine if we have VLA, or
not. */
+ tree size_constant = cp_try_fold_to_constant (size);
Again, we already called maybe_constant_value.
Sure, but maybe_constant_value still produces nops ...
If someone tries to create an array with a size that involves arithmetic
overflow, that's undefined behavior and we should probably give an
error rather than fold it away.
If we need to do some reduction to constant value here, as expr might
be actually a constant, which isn't folded here. Eg something like:
struct {
char abc[sizeof (int) * 8];
};
Due delayed folding array index isn't necessarily reduced here. So we
need to perform at least constant value folding for diagnostics, as we
do right now.
Yes, we need to do some folding, that's why we call maybe_constant_value!
@@ -13078,6 +13042,8 @@ build_enumerator (tree name, tree value,
tree
enumtype, tree attributes,
if (value)
STRIP_TYPE_NOPS (value);
+ if (value)
+ value = cp_try_fold_to_constant (value);
Again, this is unnecessary because we call cxx_constant_value below.
See nops, and other unary-operations we want to reduce here to real
constant value ...
The cxx_constant_value call below will deal with them.
Likewise for grokbitfield.
Hmm, AFAIR we don't call cxx_constant_value in all code-paths. But I
will look into it, and come back to you on it.
I am still on it ... first did the other points
Looks like this hasn't changed.
Yes, for grokbitfield current version uses fold_simple for witdth. So
just expressions based on constants getting reduced to short form. In
grokbitfield I don't see invocation of cxx_constant_value. So how can
we be sure that width is reduced to integer-cst?
We call cxx_constant_value on bit-field widths in check_bitfield_decl.
@@ -6575,6 +6578,13 @@ cp_parser_postfix_open_square_expression
(cp_parser
*parser,
index = cp_parser_expression (parser);
}
+ /* For offsetof and declaration of types we need
+ constant integeral values.
+ Also we meed to fold for negative constants so that diagnostic
in
+ c-family/c-common.c doesn't fail for array-bounds. */
+ if (for_offsetof || decltype_p
+ || (TREE_CODE (index) == NEGATE_EXPR && TREE_CODE
(TREE_OPERAND
(index, 0)) == INTEGER_CST))
+ index = cp_try_fold_to_constant (index);
Similarly, for offsetof the folding should happen closer to where it
is needed.
Why is it needed for decltype, which is querying the type of an
expression?
For NEGATE_EXPR, we had talked about always folding a NEGATE of a
constant; this isn't the right place to do it.
Same as above, we need in those cases (and for -1 too) the constant
values early anyway. So I saw it as more logical to have done this
conversion as soon as possible after initialization.
I don't think this is as soon as possible; we can fold the NEGATE_EXPR
immediately when we build it, at the end of cp_build_unary_op.
I still wonder why any folding is necessary for decltype. When I ask
why, I want to know *why*, not just have you tell me again that it's
needed. I don't think it is.
For offsetof, I wonder if it makes sense to extend fold_offsetof_1 to
handle whatever additional folding is needed here. If not, then fold
in finish_offsetof, before calling fold_offsetof.
I see that this is now an unconditional fold_simple, but I still don't
understand why it needs to be folded here, in the parser.
The point to fold the 'value' here is for cases
'processing_template_decl' isn't false. We could move it to the
else-case of the 'if (! processing_template_decl)' line for being more
explicit?
Well, on looking here in more detail, we might don't that that initial
folding here. As for processing_template_decl fold_simple (and
cp_fully_fold) doesn't do much.
Looks like the fold is still there.
Yes, but a fold_simple one just working on constant values. It
doesn't fold expressions like 'a == a' to a constant. I extended
comment in current version on branch. Additionally it invokes now the
fold_simple always.
We want to reduce index, if possible, for
diagnostics in code in c-family/c-common.c
Why not closer to the diagnostics?
for array-bounds,
We already fold array bounds.
for types (they need to be fully folded)
WHY? How many times do I need to ask you for SOME reason? You keep
just saying it's necessary without any evidence.
and to be sure we simplify basic operations on constant-values.
Why here, rather than closer to where we care about such simplification?
Again:
I want to delay it to:
1) the places where we actually care about constant values, all of
which
already call maybe_constant_value or cxx_constant_value, so they
shouldn't need much change; and
2) the places where we want a simplified expression for warnings, where
we should call fold_simple.
Folding in the parser is wrong, most of all because template
substitution doesn't go through the parser.
In 'cp_parser_omp_var_list_no_open' we need to fold 'length' can
'low_bound' as those values getting checked some lines below (see
lines 27936, 27944).
OK, but this seems like an typical case of needing to fold for diagnostics;
usually in those cases you use the folded value for the diagnostics and then
keep using the unfolded expression elsewhere.
Right.
So are you going to make that change here?
In 'cp_parser_cilk_grainsize' we fold 2nd argument of
'cp_paser_cild_for' by 'fold_simple'. Not sure if it is worth to move
operand-folding into cp_parser_cilk_for itself, as we have here just
two users of 'cp_parser_cilk_for'.
One time we pass 'integer_zero_node' as this argument, and the other
time a binary-expression, which might be constant value.
But sure we can move it into 'cp_parser_cilk_grainsize'.if you prefer?
Why does the fold need to be in the parser?
Well, if we hit it during our tree-walk in cp_fold_r, then we don't
need to fold it here. I will check, if this is really necessary.
?
@@ -7249,7 +7249,7 @@ gimplify_omp_for (tree *expr_p, gimple_seq
*pre_p)
/* Handle OMP_FOR_COND. */
t = TREE_VEC_ELT (OMP_FOR_COND (for_stmt), i);
gcc_assert (COMPARISON_CLASS_P (t));
- gcc_assert (TREE_OPERAND (t, 0) == decl);
+ gcc_assert (TREE_OPERAND (t, 0) == decl || TREE_OPERAND (t,
1) ==
decl);
Why didn't delayed folding canonicalize this so that the decl is in
op0?
Delay folding doesn't canonicalize this.
Why not? Doesn't it fold all expressions?
?
It fold them lately. I will recheck this code-change. It might be no
longer required due recent changes to omp-folding. It could be that
original pattern didn't applied here anymore, and therefore statement
didn't been transformed into its canonical form. Bit I assume this
could be resolved.
?
This hunk is necessary as we don't use canonical-form produced by
shorten_compare anymore. Therefore special operand can occur on
right-hand side too.
That seems like a problem, if the middle end is expecting the canonical
form. What is your plan for dealing with shorten_compare issues, again?
@@ -1947,6 +1947,8 @@ build_complex (tree type, tree real, tree
imag)
{
tree t = make_node (COMPLEX_CST);
+ real = fold (real);
+ imag = fold (imag);
I still think this is wrong. The arguments should be sufficiently
folded.
As we don't fold unary-operators on constants, we need to fold it at
some place. AFAICS is the C++ FE not calling directly build_complex.
So this place was the easiest way to avoid issues with things like '-'
'1' etc.
Is this because of the
value = build_complex (NULL_TREE, convert (const_type,
integer_zero_node),
value);
Might be. This should be indeed a 'fold_convert', isn't it?
Yes.
Applied modification to it.
So can we remove the fold in build_complex now?
in interpret_float? I think "convert" definitely needs to do some
folding, since it's called from middle-end code that expects that.
I remember talking about "convert" doing some folding (and cp_convert
not) in our 1:1 last week.
Can't remember that. I know that we were talking about the difference
of convert and fold_convert. convert can be used on C++ specifics,
but fold_convert is something shared with ME.
convert is called from the ME, which sometimes expects folding.
So first 'fold_convert'
isn't the same as 'fold (convert ())'.
I don't find places we invoke convert () in ME. We have some calls in
convert.c (see convert_to_integer, convert_to_integer_nofold, and
convert_to_real), which all used in AST only AFAICS.
I was thinking of convert.c and fold-const.c to be part of the ME, since
they are language-independent. But I guess other people think of the ME
starting with gimple.
And it looks like the only language-independent uses of convert are in
c-family; I guess many of them should change to fold_convert.
Hmm, in context of this work? Or is this more a general point about future work?
In the context of this work, if they are introducing problematic NOPs.
@@ -5080,6 +5081,7 @@ output_constructor_bitfield (oc_local_state
*local,
unsigned int bit_offset)
while (TREE_CODE (local->val) == VIEW_CONVERT_EXPR
|| TREE_CODE (local->val) == NON_LVALUE_EXPR)
local->val = TREE_OPERAND (local->val, 0);
+ local->val = fold (local->val);
Likewise.
As soon as we can be sure that values getting fully_folded, or at
least folded for constants, we should be able to remove this.
Yep, they need to be folded before we get here.
I didn't come to remove this line for testing. As we fold now for
initializers more early, and cp_fold supports constructors, it could
be that we don't need this anymore. It is on my pile.
That fold is still required. By removing it, I saw boostrap issue due
'invalid initializer'.
That indicates a folding problem earlier on, that will cause some
initialization that should be performed at compile time to happen at run
time instead.
Please investigate the bootstrap issue further.
@@ -5776,6 +5776,8 @@ convert_nontype_argument (tree type, tree expr,
tsubst_flags_t complain)
{
tree expr_type;
+ expr = cp_try_fold_to_constant (expr);
And here, convert_nontype_argument already uses
maybe_constant_value/cxx_constant_value for folding constants.
Yes, this invocation looks useless too. I think I introduced it for
the STRING_CST check below, but AFAICS we should assume it as
unnecessary. I will change it and do regression-testing.
Is this still in process?
By recent changes we seem to hit for c++ some additional regression.
They are related to negate-shifts for c++11. We are hitting now the
check within cxx_constant_value. The cxx_eval_check_shift_p sees now
that left-hand operand is negative and produces two new errors for
following tests: c-c++-common Wshift-negative-value-*.c cases.
Which lines are affected? Are the new messages correct or not?
So we will need adjust those cases, or invoke within this
eval-function instead maybe_constant_value to avoid that ?
This eval function is part of constexpr evaluation, so it wouldn't make
sense to call maybe_constant_value here. And the arguments to this
function have just gone through cxx_eval_constant_expression.
Jason
