Re: C++ delayed folding branch review
2015-08-28 9:19 GMT+02:00 Kai Tietz :
> 2015-08-28 4:11 GMT+02:00 Jason Merrill :
>> On 08/27/2015 02:12 PM, Kai Tietz wrote:
>>>
>>> + else if (TREE_CODE (type) == VECTOR_TYPE)
>>> +{
>>> + if (TREE_CODE (arg1) == VECTOR_CST
>>> + && code == NOP_EXPR
>>> + && TYPE_VECTOR_SUBPARTS (type) == VECTOR_CST_NELTS (arg1))
>>> + {
>>> + tree r = copy_node (arg1);
>>> + TREE_TYPE (arg1) = type;
>>> + return r;
>>> + }
>>> +}
>>
>>
>> I would drop the check on 'code' and add a check that
>>
>> TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (TREE_TYPE (arg1))
>>
>> Does that still pass?
>
> Yes, is still passes. To check here for main-variant seems to be more
> robust. I commit it to branch, and will do complete
> regression-testing for it.
Completed regression-testing. No new regressions.
Kai
Re: C++ delayed folding branch review
2015-08-28 4:11 GMT+02:00 Jason Merrill :
> On 08/27/2015 02:12 PM, Kai Tietz wrote:
>>
>> + else if (TREE_CODE (type) == VECTOR_TYPE)
>> +{
>> + if (TREE_CODE (arg1) == VECTOR_CST
>> + && code == NOP_EXPR
>> + && TYPE_VECTOR_SUBPARTS (type) == VECTOR_CST_NELTS (arg1))
>> + {
>> + tree r = copy_node (arg1);
>> + TREE_TYPE (arg1) = type;
>> + return r;
>> + }
>> +}
>
>
> I would drop the check on 'code' and add a check that
>
> TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (TREE_TYPE (arg1))
>
> Does that still pass?
Yes, is still passes. To check here for main-variant seems to be more
robust. I commit it to branch, and will do complete
regression-testing for it.
> Jason
Kai
Re: C++ delayed folding branch review
On 08/27/2015 02:12 PM, Kai Tietz wrote:
+ else if (TREE_CODE (type) == VECTOR_TYPE)
+{
+ if (TREE_CODE (arg1) == VECTOR_CST
+ && code == NOP_EXPR
+ && TYPE_VECTOR_SUBPARTS (type) == VECTOR_CST_NELTS (arg1))
+ {
+ tree r = copy_node (arg1);
+ TREE_TYPE (arg1) = type;
+ return r;
+ }
+}
I would drop the check on 'code' and add a check that
TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (TREE_TYPE (arg1))
Does that still pass?
Jason
Re: C++ delayed folding branch review
On 08/27/2015 09:38 AM, Kai Tietz wrote:
2015-08-27 15:27 GMT+02:00 Jason Merrill :
On 08/27/2015 06:39 AM, Kai Tietz wrote:
2015-08-27 4:56 GMT+02:00 Jason Merrill :
On 08/24/2015 03:15 AM, Kai Tietz wrote:
2015-08-03 17:39 GMT+02:00 Jason Merrill :
On 08/03/2015 05:42 AM, Kai Tietz wrote:
2015-08-03 5:49 GMT+02:00 Jason Merrill :
On 07/31/2015 05:54 PM, Kai Tietz wrote:
The "STRIP_NOPS-requirement in 'reduced_constant_expression_p'" I
could
remove, but for one case in constexpr. Without folding we don't do
type-sinking/raising.
Right.
So binary/unary operations might be containing cast, which were in
the
past unexpected.
Why aren't the casts folded away?
On such cast constructs, as for this vector-sample, we can't fold away
Which testcase is this?
It is the g++.dg/ext/vector20.C testcase. IIRC I mentioned this
testcase already earlier as reference, but I might be wrong here.
I don't see any casts in that testcase. So the compiler is introducing
introducing conversions back and forth between const and non-const, then?
I
suppose it doesn't so much matter where they come from, they should be
folded away regardless.
The cast gets introduced in convert.c about line 836 in function
convert_to_integer_1 AFAIK. There should be the alternative solution
for this issue by disallowing for PLUS/MINUS/... expressions the
sinking of the cast into the expression, if dofold is false, and type
has same width as inner_type, and is of vector-kind.
Why would we be calling convert_to_integer for conversions between vector
types?
the cast chain. The difference here to none-delayed-folding branch is
that the cast isn't moved out of the plus-expr. What we see now is
(plus ((vec) (const vector ...) { }), ...). Before we had (vec)
(plus (const vector ...) { ... }).
How could a PLUS_EXPR be considered a reduced constant, regardless of
where
the cast is?
Of course it is just possible to sink out a cast from PLUS_EXPR, in
pretty few circumstance (eg. on constants if both types just differ in
const-attribute, if conversion is no view-convert).
I don't understand how this is an answer to my question.
(vec) (const vector) { ... } expression can't be folded.
It currently isn't folded, but why can't we change that?
This cast to
none-const variant happens due the 'constexpr v = v +
' pattern in testcase. v is still of type vec, even
if function itself is constexpr.
I don't see that pattern in the testcase:
typedef long vec __attribute__((vector_size (2 * sizeof (long;
constexpr vec v = { 3, 4 };
constexpr vec s = v + v;
constexpr vec w = __builtin_shuffle (v, v);
If we have v + constant-value, that's because we pulled out the constant
value of one of the v's, which we ought to be doing for both of them.
On verify_constant we check by reduced_constant_expression_p, if
value
is
a constant. We don't handle here, that NOP_EXPRs are something we
want to
look through here, as it doesn't change anything if this is a
constant, or
not.
NOPs around constants should have been folded away by the time we get
there.
Not in this cases, as the we actually have here a switch from const to
none-const. So there is an attribute-change, which we can't ignore in
general.
I wasn't suggesting we ignore it, we should be able to change the type
of
the vector_cst.
Well, the vector_cst we can change type, but this wouldn't help
AFAICS. As there is still one cast surviving within PLUS_EXPR for the
other operand.
Isn't the other operand also constant? In constexpr evaluation, either
we're dealing with a bunch of constants, in which case we should be
folding
things fully, including conversions between const and non-const, or we
don't
care.
No other operand isn't a constant-value. See code-pattern in
testcase. It is of type 'vec', which isn't constant (well, 'v' is,
but constexpr doesn't know about it).
What do you mean, "constexpr doesn't know about it"?
So the way to solve it would be to move such conversion out of the
expression. For integer-scalars we do this, and for some
floating-points too. So it might be something we don't handle for
operations with vector-type.
We don't need to worry about that in constexpr evaluation, since we only
care about constant operands.
Sure, but the variable 'v' is the problem, not a constant-value itself.
But I agree that for constexpr's we could special case cast
from const to none-const (as required in expressions like const vec v
= v + 1).
Right. But really this should happen in convert.c, it shouldn't be
specific
to C++.
Hmm, maybe. But isn't one of our different goals to move such
implicit code-modification to match.pd instead?
Folding const into a constant is hardly code modification. But perhaps
it
should go into fold_unary_loc:VIEW_CONVERT_EXPR rather than into
convert.c.
Hmm, it isn't related to a view-convert. So moving it into
fold_unary_loc wouldn't solve here any
Re: C++ delayed folding branch review
With following patch testcase passes:
Index: fold-const.c
===
--- fold-const.c(Revision 227111)
+++ fold-const.c(Arbeitskopie)
@@ -2110,6 +2110,17 @@ fold_convert_const (enum tree_code code, tree type
else if (TREE_CODE (arg1) == REAL_CST)
return fold_convert_const_fixed_from_real (type, arg1);
}
+ else if (TREE_CODE (type) == VECTOR_TYPE)
+{
+ if (TREE_CODE (arg1) == VECTOR_CST
+ && code == NOP_EXPR
+ && TYPE_VECTOR_SUBPARTS (type) == VECTOR_CST_NELTS (arg1))
+ {
+ tree r = copy_node (arg1);
+ TREE_TYPE (arg1) = type;
+ return r;
+ }
+}
return NULL_TREE;
}
Index: cp/constexpr.c
===
--- constexpr.c (Revision 227111)
+++ constexpr.c (Arbeitskopie)
@@ -1441,8 +1441,6 @@ cxx_eval_call_expression (const constexpr_ctx *ctx
bool
reduced_constant_expression_p (tree t)
{
- /* Make sure we remove useless initial NOP_EXPRs. */
- STRIP_NOPS (t);
switch (TREE_CODE (t))
{
case PTRMEM_CST:
Re: C++ delayed folding branch review
2015-08-27 15:27 GMT+02:00 Jason Merrill :
> On 08/27/2015 06:39 AM, Kai Tietz wrote:
>>
>> 2015-08-27 4:56 GMT+02:00 Jason Merrill :
>>>
>>> On 08/24/2015 03:15 AM, Kai Tietz wrote:
2015-08-03 17:39 GMT+02:00 Jason Merrill :
>
> On 08/03/2015 05:42 AM, Kai Tietz wrote:
>>
>> 2015-08-03 5:49 GMT+02:00 Jason Merrill :
>>>
>>> On 07/31/2015 05:54 PM, Kai Tietz wrote:
The "STRIP_NOPS-requirement in 'reduced_constant_expression_p'" I
could
remove, but for one case in constexpr. Without folding we don't do
type-sinking/raising.
>>>
>>>
>>>
>>> Right.
>>>
So binary/unary operations might be containing cast, which were in
the
past unexpected.
>>>
>>>
>>>
>>> Why aren't the casts folded away?
>>
>>
>>
>> On such cast constructs, as for this vector-sample, we can't fold away
>
>
>
> Which testcase is this?
It is the g++.dg/ext/vector20.C testcase. IIRC I mentioned this
testcase already earlier as reference, but I might be wrong here.
>>>
>>>
>>>
>>> I don't see any casts in that testcase. So the compiler is introducing
>>> introducing conversions back and forth between const and non-const, then?
>>> I
>>> suppose it doesn't so much matter where they come from, they should be
>>> folded away regardless.
>>
>>
>> The cast gets introduced in convert.c about line 836 in function
>> convert_to_integer_1 AFAIK. There should be the alternative solution
>> for this issue by disallowing for PLUS/MINUS/... expressions the
>> sinking of the cast into the expression, if dofold is false, and type
>> has same width as inner_type, and is of vector-kind.
>
>
> Why would we be calling convert_to_integer for conversions between vector
> types?
>
>> the cast chain. The difference here to none-delayed-folding branch is
>> that the cast isn't moved out of the plus-expr. What we see now is
>> (plus ((vec) (const vector ...) { }), ...). Before we had (vec)
>> (plus (const vector ...) { ... }).
>
>
>
> How could a PLUS_EXPR be considered a reduced constant, regardless of
> where
> the cast is?
Of course it is just possible to sink out a cast from PLUS_EXPR, in
pretty few circumstance (eg. on constants if both types just differ in
const-attribute, if conversion is no view-convert).
>>>
>>>
>>>
>>> I don't understand how this is an answer to my question.
>>
>>
>> (vec) (const vector) { ... } expression can't be folded.
>
>
> It currently isn't folded, but why can't we change that?
>
>> This cast to
>> none-const variant happens due the 'constexpr v = v +
>> ' pattern in testcase. v is still of type vec, even
>> if function itself is constexpr.
>
>
> I don't see that pattern in the testcase:
>
> typedef long vec __attribute__((vector_size (2 * sizeof (long;
> constexpr vec v = { 3, 4 };
> constexpr vec s = v + v;
> constexpr vec w = __builtin_shuffle (v, v);
>
> If we have v + constant-value, that's because we pulled out the constant
> value of one of the v's, which we ought to be doing for both of them.
>
On verify_constant we check by reduced_constant_expression_p, if
value
is
a constant. We don't handle here, that NOP_EXPRs are something we
want to
look through here, as it doesn't change anything if this is a
constant, or
not.
>>>
>>>
>>>
>>> NOPs around constants should have been folded away by the time we get
>>> there.
>>
>>
>>
>> Not in this cases, as the we actually have here a switch from const to
>> none-const. So there is an attribute-change, which we can't ignore in
>> general.
>
>
>
> I wasn't suggesting we ignore it, we should be able to change the type
> of
> the vector_cst.
Well, the vector_cst we can change type, but this wouldn't help
AFAICS. As there is still one cast surviving within PLUS_EXPR for the
other operand.
>>>
>>>
>>>
>>> Isn't the other operand also constant? In constexpr evaluation, either
>>> we're dealing with a bunch of constants, in which case we should be
>>> folding
>>> things fully, including conversions between const and non-const, or we
>>> don't
>>> care.
>>
>>
>> No other operand isn't a constant-value. See code-pattern in
>> testcase. It is of type 'vec', which isn't constant (well, 'v' is,
>> but constexpr doesn't know about it).
>
>
> What do you mean, "constexpr doesn't know about it"?
>
So the way to solve it would be to move such conversion out of the
expression. For integer-scalars we do this, and for some
floating-points too. So it might be something we don't handle for
operations with vector-type.
>>>
>>>
>>>
>>> We don't need to worry about that in constexpr evaluation, since we only
>>> care abou
Re: C++ delayed folding branch review
On 08/27/2015 06:39 AM, Kai Tietz wrote:
2015-08-27 4:56 GMT+02:00 Jason Merrill :
On 08/24/2015 03:15 AM, Kai Tietz wrote:
2015-08-03 17:39 GMT+02:00 Jason Merrill :
On 08/03/2015 05:42 AM, Kai Tietz wrote:
2015-08-03 5:49 GMT+02:00 Jason Merrill :
On 07/31/2015 05:54 PM, Kai Tietz wrote:
The "STRIP_NOPS-requirement in 'reduced_constant_expression_p'" I
could
remove, but for one case in constexpr. Without folding we don't do
type-sinking/raising.
Right.
So binary/unary operations might be containing cast, which were in the
past unexpected.
Why aren't the casts folded away?
On such cast constructs, as for this vector-sample, we can't fold away
Which testcase is this?
It is the g++.dg/ext/vector20.C testcase. IIRC I mentioned this
testcase already earlier as reference, but I might be wrong here.
I don't see any casts in that testcase. So the compiler is introducing
introducing conversions back and forth between const and non-const, then? I
suppose it doesn't so much matter where they come from, they should be
folded away regardless.
The cast gets introduced in convert.c about line 836 in function
convert_to_integer_1 AFAIK. There should be the alternative solution
for this issue by disallowing for PLUS/MINUS/... expressions the
sinking of the cast into the expression, if dofold is false, and type
has same width as inner_type, and is of vector-kind.
Why would we be calling convert_to_integer for conversions between
vector types?
the cast chain. The difference here to none-delayed-folding branch is
that the cast isn't moved out of the plus-expr. What we see now is
(plus ((vec) (const vector ...) { }), ...). Before we had (vec)
(plus (const vector ...) { ... }).
How could a PLUS_EXPR be considered a reduced constant, regardless of
where
the cast is?
Of course it is just possible to sink out a cast from PLUS_EXPR, in
pretty few circumstance (eg. on constants if both types just differ in
const-attribute, if conversion is no view-convert).
I don't understand how this is an answer to my question.
(vec) (const vector) { ... } expression can't be folded.
It currently isn't folded, but why can't we change that?
This cast to
none-const variant happens due the 'constexpr v = v +
' pattern in testcase. v is still of type vec, even
if function itself is constexpr.
I don't see that pattern in the testcase:
typedef long vec __attribute__((vector_size (2 * sizeof (long;
constexpr vec v = { 3, 4 };
constexpr vec s = v + v;
constexpr vec w = __builtin_shuffle (v, v);
If we have v + constant-value, that's because we pulled out the constant
value of one of the v's, which we ought to be doing for both of them.
On verify_constant we check by reduced_constant_expression_p, if value
is
a constant. We don't handle here, that NOP_EXPRs are something we
want to
look through here, as it doesn't change anything if this is a
constant, or
not.
NOPs around constants should have been folded away by the time we get
there.
Not in this cases, as the we actually have here a switch from const to
none-const. So there is an attribute-change, which we can't ignore in
general.
I wasn't suggesting we ignore it, we should be able to change the type of
the vector_cst.
Well, the vector_cst we can change type, but this wouldn't help
AFAICS. As there is still one cast surviving within PLUS_EXPR for the
other operand.
Isn't the other operand also constant? In constexpr evaluation, either
we're dealing with a bunch of constants, in which case we should be folding
things fully, including conversions between const and non-const, or we don't
care.
No other operand isn't a constant-value. See code-pattern in
testcase. It is of type 'vec', which isn't constant (well, 'v' is,
but constexpr doesn't know about it).
What do you mean, "constexpr doesn't know about it"?
So the way to solve it would be to move such conversion out of the
expression. For integer-scalars we do this, and for some
floating-points too. So it might be something we don't handle for
operations with vector-type.
We don't need to worry about that in constexpr evaluation, since we only
care about constant operands.
Sure, but the variable 'v' is the problem, not a constant-value itself.
But I agree that for constexpr's we could special case cast
from const to none-const (as required in expressions like const vec v
= v + 1).
Right. But really this should happen in convert.c, it shouldn't be
specific
to C++.
Hmm, maybe. But isn't one of our different goals to move such
implicit code-modification to match.pd instead?
Folding const into a constant is hardly code modification. But perhaps it
should go into fold_unary_loc:VIEW_CONVERT_EXPR rather than into convert.c.
Hmm, it isn't related to a view-convert. So moving it into
fold_unary_loc wouldn't solve here anything. Issue is in constexpr
code, not in folding itself.
What TREE_CODE does the conversion (vec) (const vector)
Re: C++ delayed folding branch review
2015-08-27 4:56 GMT+02:00 Jason Merrill :
> On 08/24/2015 03:15 AM, Kai Tietz wrote:
>>
>> 2015-08-03 17:39 GMT+02:00 Jason Merrill :
>>>
>>> On 08/03/2015 05:42 AM, Kai Tietz wrote:
2015-08-03 5:49 GMT+02:00 Jason Merrill :
>
> On 07/31/2015 05:54 PM, Kai Tietz wrote:
>>
>>
>> The "STRIP_NOPS-requirement in 'reduced_constant_expression_p'" I
>> could
>> remove, but for one case in constexpr. Without folding we don't do
>> type-sinking/raising.
>
>
> Right.
>
>> So binary/unary operations might be containing cast, which were in the
>> past unexpected.
>
>
> Why aren't the casts folded away?
On such cast constructs, as for this vector-sample, we can't fold away
>>>
>>>
>>> Which testcase is this?
>>
>>
>> It is the g++.dg/ext/vector20.C testcase. IIRC I mentioned this
>> testcase already earlier as reference, but I might be wrong here.
>
>
> I don't see any casts in that testcase. So the compiler is introducing
> introducing conversions back and forth between const and non-const, then? I
> suppose it doesn't so much matter where they come from, they should be
> folded away regardless.
The cast gets introduced in convert.c about line 836 in function
convert_to_integer_1 AFAIK. There should be the alternative solution
for this issue by disallowing for PLUS/MINUS/... expressions the
sinking of the cast into the expression, if dofold is false, and type
has same width as inner_type, and is of vector-kind.
the cast chain. The difference here to none-delayed-folding branch is
that the cast isn't moved out of the plus-expr. What we see now is
(plus ((vec) (const vector ...) { }), ...). Before we had (vec)
(plus (const vector ...) { ... }).
>>>
>>>
>>> How could a PLUS_EXPR be considered a reduced constant, regardless of
>>> where
>>> the cast is?
>>
>>
>> Of course it is just possible to sink out a cast from PLUS_EXPR, in
>> pretty few circumstance (eg. on constants if both types just differ in
>> const-attribute, if conversion is no view-convert).
>
>
> I don't understand how this is an answer to my question.
(vec) (const vector) { ... } expression can't be folded. This cast to
none-const variant happens due the 'constexpr v = v +
' pattern in testcase. v is still of type vec, even
if function itself is constexpr.
>> On verify_constant we check by reduced_constant_expression_p, if value
>> is
>> a constant. We don't handle here, that NOP_EXPRs are something we
>> want to
>> look through here, as it doesn't change anything if this is a
>> constant, or
>> not.
>
>
> NOPs around constants should have been folded away by the time we get
> there.
Not in this cases, as the we actually have here a switch from const to
none-const. So there is an attribute-change, which we can't ignore in
general.
>>>
>>>
>>> I wasn't suggesting we ignore it, we should be able to change the type of
>>> the vector_cst.
>>
>>
>> Well, the vector_cst we can change type, but this wouldn't help
>> AFAICS. As there is still one cast surviving within PLUS_EXPR for the
>> other operand.
>
>
> Isn't the other operand also constant? In constexpr evaluation, either
> we're dealing with a bunch of constants, in which case we should be folding
> things fully, including conversions between const and non-const, or we don't
> care.
No other operand isn't a constant-value. See code-pattern in
testcase. It is of type 'vec', which isn't constant (well, 'v' is,
but constexpr doesn't know about it).
The bogus error-message happens in:
#1 0x00668c20 in verify_constant (t=t@entry=0xffd3cbe8,
allow_non_constant=,
non_constant_p=non_constant_p@entry=0xe5fa6fa,
overflow_p=overflow_p@entry=0xe5fa6fb)
at ../../src/gcc/cp/constexpr.c:1480
#2 0x0066c710 in cxx_eval_binary_expression (overflow_p=0xe5fa6fb,
non_constant_p=0xe5fa6fa, t=0xffd3cba0, ctx=0xe5fa6fc)
at ../../src/gcc/cp/constexpr.c:1620
#3 cxx_eval_constant_expression (ctx=ctx@entry=0xe5fa6fc,
t=t@entry=0xffd3cba0, lval=lval@entry=false,
non_constant_p=non_constant_p@entry=0xe5fa6fa,
overflow_p=overflow_p@entry=0xe5fa6fb, jump_target=jump_target@entry=0x0)
at ../../src/gcc/cp/constexpr.c:3491
#2 0x0066c710 in cxx_eval_binary_expression (overflow_p=0xe5fa6fb,
non_constant_p=0xe5fa6fa, t=0xffd3cba0, ctx=0xe5fa6fc)
at ../../src/gcc/cp/constexpr.c:1620
1620VERIFY_CONSTANT (lhs);
>> So the way to solve it would be to move such conversion out of the
>> expression. For integer-scalars we do this, and for some
>> floating-points too. So it might be something we don't handle for
>> operations with vector-type.
>
>
> We don't need to worry about that in constexpr evaluation, since we only
> care about constant operands.
Sure, but the variable 'v' is the problem, not a constant-value itself.
But I agree that for constexpr's we could special case
Re: C++ delayed folding branch review
On 08/24/2015 03:15 AM, Kai Tietz wrote:
2015-08-03 17:39 GMT+02:00 Jason Merrill :
On 08/03/2015 05:42 AM, Kai Tietz wrote:
2015-08-03 5:49 GMT+02:00 Jason Merrill :
On 07/31/2015 05:54 PM, Kai Tietz wrote:
The "STRIP_NOPS-requirement in 'reduced_constant_expression_p'" I could
remove, but for one case in constexpr. Without folding we don't do
type-sinking/raising.
Right.
So binary/unary operations might be containing cast, which were in the
past unexpected.
Why aren't the casts folded away?
On such cast constructs, as for this vector-sample, we can't fold away
Which testcase is this?
It is the g++.dg/ext/vector20.C testcase. IIRC I mentioned this
testcase already earlier as reference, but I might be wrong here.
I don't see any casts in that testcase. So the compiler is introducing
introducing conversions back and forth between const and non-const,
then? I suppose it doesn't so much matter where they come from, they
should be folded away regardless.
the cast chain. The difference here to none-delayed-folding branch is
that the cast isn't moved out of the plus-expr. What we see now is
(plus ((vec) (const vector ...) { }), ...). Before we had (vec)
(plus (const vector ...) { ... }).
How could a PLUS_EXPR be considered a reduced constant, regardless of where
the cast is?
Of course it is just possible to sink out a cast from PLUS_EXPR, in
pretty few circumstance (eg. on constants if both types just differ in
const-attribute, if conversion is no view-convert).
I don't understand how this is an answer to my question.
On verify_constant we check by reduced_constant_expression_p, if value is
a constant. We don't handle here, that NOP_EXPRs are something we want to
look through here, as it doesn't change anything if this is a constant, or
not.
NOPs around constants should have been folded away by the time we get
there.
Not in this cases, as the we actually have here a switch from const to
none-const. So there is an attribute-change, which we can't ignore in
general.
I wasn't suggesting we ignore it, we should be able to change the type of
the vector_cst.
Well, the vector_cst we can change type, but this wouldn't help
AFAICS. As there is still one cast surviving within PLUS_EXPR for the
other operand.
Isn't the other operand also constant? In constexpr evaluation, either
we're dealing with a bunch of constants, in which case we should be
folding things fully, including conversions between const and non-const,
or we don't care.
So the way to solve it would be to move such conversion out of the
expression. For integer-scalars we do this, and for some
floating-points too. So it might be something we don't handle for
operations with vector-type.
We don't need to worry about that in constexpr evaluation, since we only
care about constant operands.
But I agree that for constexpr's we could special case cast
from const to none-const (as required in expressions like const vec v
= v + 1).
Right. But really this should happen in convert.c, it shouldn't be specific
to C++.
Hmm, maybe. But isn't one of our different goals to move such
implicit code-modification to match.pd instead?
Folding const into a constant is hardly code modification. But perhaps
it should go into fold_unary_loc:VIEW_CONVERT_EXPR rather than into
convert.c.
Jason
Re: C++ delayed folding branch review
Hello Jason,
after a longer delay the answer to your question.
2015-08-03 17:39 GMT+02:00 Jason Merrill :
> On 08/03/2015 05:42 AM, Kai Tietz wrote:
>>
>> 2015-08-03 5:49 GMT+02:00 Jason Merrill :
>>>
>>> On 07/31/2015 05:54 PM, Kai Tietz wrote:
The "STRIP_NOPS-requirement in 'reduced_constant_expression_p'" I could
remove, but for one case in constexpr. Without folding we don't do
type-sinking/raising.
>>>
>>>
>>>
>>> Right.
>>>
So binary/unary operations might be containing cast, which were in the
past unexpected.
>>>
>>>
>>> Why aren't the casts folded away?
>>
>>
>> On such cast constructs, as for this vector-sample, we can't fold away
>
>
> Which testcase is this?
It is the g++.dg/ext/vector20.C testcase. IIRC I mentioned this
testcase already earlier as reference, but I might be wrong here.
>> the cast chain. The difference here to none-delayed-folding branch is
>> that the cast isn't moved out of the plus-expr. What we see now is
>> (plus ((vec) (const vector ...) { }), ...). Before we had (vec)
>> (plus (const vector ...) { ... }).
>
>
> How could a PLUS_EXPR be considered a reduced constant, regardless of where
> the cast is?
Of course it is just possible to sink out a cast from PLUS_EXPR, in
pretty few circumstance (eg. on constants if both types just differ in
const-attribute, if conversion is no view-convert).
On verify_constant we check by reduced_constant_expression_p, if value
is
a constant. We don't handle here, that NOP_EXPRs are something we want
to
look through here, as it doesn't change anything if this is a constant,
or
not.
>>>
>>>
>>> NOPs around constants should have been folded away by the time we get
>>> there.
>>
>>
>> Not in this cases, as the we actually have here a switch from const to
>> none-const. So there is an attribute-change, which we can't ignore in
>> general.
>
>
> I wasn't suggesting we ignore it, we should be able to change the type of
> the vector_cst.
Well, the vector_cst we can change type, but this wouldn't help
AFAICS. As there is still one cast surviving within PLUS_EXPR for the
other operand.
So the way to solve it would be to move such conversion out of the
expression. For integer-scalars we do this, and for some
floating-points too. So it might be something we don't handle for
operations with vector-type.
>> But I agree that for constexpr's we could special case cast
>> from const to none-const (as required in expressions like const vec v
>> = v + 1).
>
>
> Right. But really this should happen in convert.c, it shouldn't be specific
> to C++.
Hmm, maybe. But isn't one of our different goals to move such
implicit code-modification to match.pd instead?
> Jason
>
Kai
Re: C++ delayed folding branch review
On 08/03/2015 05:42 AM, Kai Tietz wrote:
2015-08-03 5:49 GMT+02:00 Jason Merrill :
On 07/31/2015 05:54 PM, Kai Tietz wrote:
The "STRIP_NOPS-requirement in 'reduced_constant_expression_p'" I could
remove, but for one case in constexpr. Without folding we don't do
type-sinking/raising.
Right.
So binary/unary operations might be containing cast, which were in the
past unexpected.
Why aren't the casts folded away?
On such cast constructs, as for this vector-sample, we can't fold away
Which testcase is this?
the cast chain. The difference here to none-delayed-folding branch is
that the cast isn't moved out of the plus-expr. What we see now is
(plus ((vec) (const vector ...) { }), ...). Before we had (vec)
(plus (const vector ...) { ... }).
How could a PLUS_EXPR be considered a reduced constant, regardless of
where the cast is?
On verify_constant we check by reduced_constant_expression_p, if value is
a constant. We don't handle here, that NOP_EXPRs are something we want to
look through here, as it doesn't change anything if this is a constant, or
not.
NOPs around constants should have been folded away by the time we get there.
Not in this cases, as the we actually have here a switch from const to
none-const. So there is an attribute-change, which we can't ignore in
general.
I wasn't suggesting we ignore it, we should be able to change the type
of the vector_cst.
But I agree that for constexpr's we could special case cast
from const to none-const (as required in expressions like const vec v
= v + 1).
Right. But really this should happen in convert.c, it shouldn't be
specific to C++.
Jason
Re: C++ delayed folding branch review
2015-08-03 5:49 GMT+02:00 Jason Merrill :
> On 07/31/2015 05:54 PM, Kai Tietz wrote:
>>
>> The "STRIP_NOPS-requirement in 'reduced_constant_expression_p'" I could
>> remove, but for one case in constexpr. Without folding we don't do
>> type-sinking/raising.
>
>
> Right.
>
>> So binary/unary operations might be containing cast, which were in the
>> past unexpected.
>
>
> Why aren't the casts folded away?
On such cast constructs, as for this vector-sample, we can't fold away
the cast chain. The difference here to none-delayed-folding branch is
that the cast isn't moved out of the plus-expr. What we see now is
(plus ((vec) (const vector ...) { }), ...). Before we had (vec)
(plus (const vector ...) { ... }).
>> On verify_constant we check by reduced_constant_expression_p, if value is
>> a constant. We don't handle here, that NOP_EXPRs are something we want to
>> look through here, as it doesn't change anything if this is a constant, or
>> not.
>
>
> NOPs around constants should have been folded away by the time we get there.
Not in this cases, as the we actually have here a switch from const to
none-const. So there is an attribute-change, which we can't ignore in
general. But I agree that for constexpr's we could special case cast
from const to none-const (as required in expressions like const vec v
= v + 1).
> Jason
>
Kai
Re: C++ delayed folding branch review
On 07/31/2015 05:54 PM, Kai Tietz wrote: The "STRIP_NOPS-requirement in 'reduced_constant_expression_p'" I could remove, but for one case in constexpr. Without folding we don't do type-sinking/raising. Right. So binary/unary operations might be containing cast, which were in the past unexpected. Why aren't the casts folded away? On verify_constant we check by reduced_constant_expression_p, if value is a constant. We don't handle here, that NOP_EXPRs are something we want to look through here, as it doesn't change anything if this is a constant, or not. NOPs around constants should have been folded away by the time we get there. Jason
Re: C++ delayed folding branch review
- Ursprüngliche Mail -
> On 07/30/2015 05:00 PM, Kai Tietz wrote:
> > 2015-07-30 18:52 GMT+02:00 Jason Merrill :
> >> On 07/29/2015 06:56 PM, Kai Tietz wrote:
> >>> @@ -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.
> >
> > Hmm, ok. But I don't see that this function gets called in context of
> > grokbitfield, after we set DECL_INITIAL.
>
> Nope, it's called later on as part of finish_struct.
>
Ok, adjusted.
> > By removing this folding here, we get new failures in
> > g++.dg/warn/overflow-warn-1.C testcase:
> > New errors are at lin 32 that 'bif-foeld 's::' width not an
> > integer constant'
> > and at same line ''(1 / 0) is not a constant expression'. Those
> > message don't look wrong.
> >
> > The testcase next to this 'overflow-warn-3.C and overflow-warn-4.C'
> > failing in the same manner for (1 / 0) case. But there are no other
> > regressions in g++.dg & libstdc++
> >
> > Shall I extend the testcases for this message?
>
> Please.
Done.
> >>> @@ -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?
>
Yes. Done.
>
> 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.
> >
> > Ok, I will take a closer look to convert (
Re: C++ delayed folding branch review
- Ursprüngliche Mail - > On 07/31/2015 12:46 PM, Jakub Jelinek wrote: > > On Fri, Jul 31, 2015 at 06:25:57PM +0200, Kai Tietz wrote: > >> 2015-07-31 18:14 GMT+02:00 Jason Merrill : > >>> On 07/30/2015 10:48 PM, Jeff Law wrote: > > Note, anything outside of the C/C++ front-ends depending on that > canonicalization done by shorten_compare is, IMHO, broken. > >>> > >>> I think the OMP code isn't relying on it being done by shorten_compare; > >>> it's > >>> trying to do it itself in c_finish_omp_for but is somehow thwarted by > >>> delayed folding. > >> > >> Well, this seems to be reasoned by finish_omp_for (), which gets > >> invoked in parser.c cp_parser_omp_for_loop, and/or pt.c: tsubst_expr. > >> In all those cases arguments aren't folded anymore. So > >> c_finish_omp_for's patterns don't match anymore. I guess we might > >> want to do this cannonical form in genericize-pass? > > > > Or just fold in finish_omp_for before calling c_finish_omp_for, so that it > > is in the expected form? > > That certainly sounds simpler. > > Jason Well, it sounds easier. We need to do here this special COND-folding (means operands only), and pass this into c_finish_omp. Of course we should just fold OMP_FOR's COND part. I will try a patch for this- Kai
Re: C++ delayed folding branch review
On 07/31/2015 12:46 PM, Jakub Jelinek wrote: On Fri, Jul 31, 2015 at 06:25:57PM +0200, Kai Tietz wrote: 2015-07-31 18:14 GMT+02:00 Jason Merrill : On 07/30/2015 10:48 PM, Jeff Law wrote: Note, anything outside of the C/C++ front-ends depending on that canonicalization done by shorten_compare is, IMHO, broken. I think the OMP code isn't relying on it being done by shorten_compare; it's trying to do it itself in c_finish_omp_for but is somehow thwarted by delayed folding. Well, this seems to be reasoned by finish_omp_for (), which gets invoked in parser.c cp_parser_omp_for_loop, and/or pt.c: tsubst_expr. In all those cases arguments aren't folded anymore. So c_finish_omp_for's patterns don't match anymore. I guess we might want to do this cannonical form in genericize-pass? Or just fold in finish_omp_for before calling c_finish_omp_for, so that it is in the expected form? That certainly sounds simpler. Jason
Re: C++ delayed folding branch review
On Fri, Jul 31, 2015 at 06:25:57PM +0200, Kai Tietz wrote: > 2015-07-31 18:14 GMT+02:00 Jason Merrill : > > On 07/30/2015 10:48 PM, Jeff Law wrote: > >> > >> Note, anything outside of the C/C++ front-ends depending on that > >> canonicalization done by shorten_compare is, IMHO, broken. > > > > > > I think the OMP code isn't relying on it being done by shorten_compare; it's > > trying to do it itself in c_finish_omp_for but is somehow thwarted by > > delayed folding. > > > > Jason > > > > Well, this seems to be reasoned by finish_omp_for (), which gets > invoked in parser.c cp_parser_omp_for_loop, and/or pt.c: tsubst_expr. > In all those cases arguments aren't folded anymore. So > c_finish_omp_for's patterns don't match anymore. I guess we might > want to do this cannonical form in genericize-pass? Or just fold in finish_omp_for before calling c_finish_omp_for, so that it is in the expected form? Jakub
Re: C++ delayed folding branch review
2015-07-31 18:14 GMT+02:00 Jason Merrill : > On 07/30/2015 10:48 PM, Jeff Law wrote: >> >> Note, anything outside of the C/C++ front-ends depending on that >> canonicalization done by shorten_compare is, IMHO, broken. > > > I think the OMP code isn't relying on it being done by shorten_compare; it's > trying to do it itself in c_finish_omp_for but is somehow thwarted by > delayed folding. > > Jason > Well, this seems to be reasoned by finish_omp_for (), which gets invoked in parser.c cp_parser_omp_for_loop, and/or pt.c: tsubst_expr. In all those cases arguments aren't folded anymore. So c_finish_omp_for's patterns don't match anymore. I guess we might want to do this cannonical form in genericize-pass? Kai
Re: C++ delayed folding branch review
On 07/30/2015 10:48 PM, Jeff Law wrote: Note, anything outside of the C/C++ front-ends depending on that canonicalization done by shorten_compare is, IMHO, broken. I think the OMP code isn't relying on it being done by shorten_compare; it's trying to do it itself in c_finish_omp_for but is somehow thwarted by delayed folding. Jason
Re: C++ delayed folding branch review
On 07/30/2015 05:02 PM, Jason Merrill wrote: Actually ME deals with none-cannonical form too. It just asserts on it at this place. After delayed-folding work I will continue work (Jeff pushed first parts of this work already to ML) on eliminating use of shorten_compare completely, and move its folding-patterns to match.pd. It looks like c_finish_omp_for should have done this canonicalization for the condition. And various places assume that the OMP for condition has had this canonicalization done, this is just the only place there's an assert. We need to make sure that it's done, somehow. One possibility would be to try to get Richi to accept the canonicalization patterns to match.pd without removing the equivalent code from shorten_compare. That might fix the problem y'all are running into without regressing on the warning that I ran into (I haven't tested that). Thoughts? jeff
Re: C++ delayed folding branch review
On 07/30/2015 10:52 AM, Jason Merrill wrote: 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? We want to handle the shorten_compare stuff independently of delayed folding if at all possible. It's a bit of a rats nest. One of the general problems we have is that shorten_compare also does canonicalization and issues warnings. If we're no longer getting into shorten_compare for some code, then that canonicalization isn't being done. Note, anything outside of the C/C++ front-ends depending on that canonicalization done by shorten_compare is, IMHO, broken. I've extracted a patch from Kai's shorten_compare work to move the canonicalization into match.pd *but* that runs into testsuite regressions because shorten_compare is also where we emit certain warnings for comparisons that are always true/false. With the canonicalization moved to match.pd, shorten_compare no longer recognizes a particular sequence and we lose the warning. I haven't yet found a good place to relocate that warning. This has been pushed down several items in my TODO stack. Jeff
Re: C++ delayed folding branch review
On 07/30/2015 05:00 PM, Kai Tietz wrote:
2015-07-30 18:52 GMT+02:00 Jason Merrill :
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.
See below for this. This might be related to the store_init_value issue.
@@ -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!
...so we shouldn't need cp_fully_fold.
@@ -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.
Hmm, ok. But I don't see that this function gets called in context of
grokbitfield, after we set DECL_INITIAL.
Nope, it's called later on as part of finish_struct.
By removing this folding here, we get new failures in
g++.dg/warn/overflow-warn-1.C testcase:
New errors are at lin 32 that 'bif-foeld 's::' width not an
integer constant'
and at same line ''(1 / 0) is not a constant expression'. Those
message don't look wrong.
The testcase next to this 'overflow-warn-3.C and overflow-warn-4.C'
failing in the same manner for (1 / 0) case. But there are no other
regressions in g++.dg & libstdc++
Shall I extend the testcases for this message?
Please.
@@ -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
Re: C++ delayed folding branch review
2015-07-30 18:52 GMT+02:00 Jason Merrill :
> 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.
See below for this. This might be related to the store_init_value issue.
>> 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.
Hmm, right. sizeof's type is always size_t. Its operand would make
here a difference, but this I don't check. I will remove it and test.
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 cha
Re: C++ delayed folding branch review
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; t
Re: C++ delayed folding branch review
2015-07-30 0:56 GMT+02:00 Kai Tietz :
> 2015-07-29 19:48 GMT+02:00 Jason Merrill :
>> On 07/28/2015 04:10 PM, Kai Tietz wrote:
> The change to adjust_temp_type seems to be no more necessary (just
> doing tests on it).
Yes, committed it.
>>
> @@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const
> constexpr_ctx
> *ctx, tree t,
>case CONVERT_EXPR:
>case VIEW_CONVERT_EXPR:
>case NOP_EXPR:
> +case UNARY_PLUS_EXPR:
> {
> + enum tree_code tcode = TREE_CODE (t);
> tree oldop = TREE_OPERAND (t, 0);
> +
> + if (tcode == NOP_EXPR && TREE_TYPE (t) == TREE_TYPE (oldop)
> &&
> TREE_OVERFLOW_P (oldop))
> + {
> + if (!ctx->quiet)
> + permerror (input_location, "overflow in constant
> expression");
> + /* If we're being permissive (and are in an enforcing
> + context), ignore the overflow. */
> + if (!flag_permissive)
> + *overflow_p = true;
> + *non_constant_p = true;
> +
> + return t;
> + }
> tree op = cxx_eval_constant_expression (ctx, oldop,
Why doesn't the call to cxx_eval_constant_expression at the bottom
here
handle oldop having TREE_OVERFLOW set?
>>>
>>>
>>>
>>> I just handled the case that we see here a wrapping NOP_EXPR around an
>>> overflow. As this isn't handled by cxx_eval_constant_expression.
>>
>>
>>
>> How does it need to be handled? A NOP_EXPR wrapped around an overflow
>> is there to indicated that the expression is non-constant, and it can't
>> be simplified any farther.
>>
>> Please give an example of what was going wrong.
>>
>> ^
>
> I did some regression-testing on it. This looks to me like something
> I missed to cleanup. Most changes within constexpr-code aren't
> necessary anymore. But looking on that, I think I papered over some
> issues I had about double-reporting of non-constant expression on
> overflows.
Committed change to branch for removing this.
> @@ -565,6 +571,23 @@ cp_gimplify_expr (tree *expr_p, gimple_seq
> *pre_p,
> gimple_seq *post_p)
>
> switch (code)
>{
> +case SIZEOF_EXPR:
> + if (SIZEOF_EXPR_TYPE_P (*expr_p))
> + *expr_p = cxx_sizeof_or_alignof_type (TREE_TYPE
> (TREE_OPERAND
> (*expr_p,
> +
> 0)),
> + SIZEOF_EXPR, false);
> + else if (TYPE_P (TREE_OPERAND (*expr_p, 0)))
> + *expr_p = cxx_sizeof_or_alignof_type (TREE_OPERAND (*expr_p,
> 0),
> + SIZEOF_EXPR, false);
> + else
> + *expr_p = cxx_sizeof_or_alignof_expr (TREE_OPERAND (*expr_p,
> 0),
> + SIZEOF_EXPR, false);
> + if (*expr_p == error_mark_node)
> + *expr_p = size_one_node;
> +
> + *expr_p = maybe_constant_value (*expr_p);
> + ret = GS_OK;
> + break;
Why are these surviving until gimplification time?
>>>
>>>
>>> This might be still necessary. I will retest, when bootstrap works.
>>> As we now added SIZEOF_EXPR folding to cp_fold, and if we catch all
>>> expressions a sizeof can occure, this shouldn't be necessary anymore.
>>> AFAIR I saw here some issues about initialzation for global-variables,
>>> which weren't caught.
>>
>>
>>
>> Hmm, I wonder why you would see issues with global initializers that
>> aren't seen on trunk? In any case, if the issue is with global
>> initializers, they should be handled sooner, not here.
>>
>
> They don't survice in function-context, but outside they might. On
> trunk we never will see an sizeof-expression in such case as they got
> folded-away much earlier.
>
> I will try an bootstrap with disabling it. In ME we don't produce
> sizeof-expressions anymore, so we don't need to think about
> re-gimplifiying some AST AFAICS.
Tested. We seems not to need the handle of SIZEOF_EXPR in gimplifier
anymore. so removed hunk.
>>
> @@ -1529,8 +1532,11 @@ build_expr_type_conversion (int desires, tree
> expr,
> bool complain)
> tree basetype = TREE_TYPE (expr);
> tree conv = NULL_TREE;
> tree winner = NULL_TREE;
> + /* Want to see if EXPR is a constant. See below checks for
> null_node.
> */
> + tree expr_folded = cp_try_fold_to_constant (expr);
>
> - if (expr == null_
Re: C++ delayed folding branch review
2015-07-29 19:48 GMT+02:00 Jason Merrill :
> On 07/28/2015 04:10 PM, Kai Tietz wrote:
>>
>> 2015-07-28 1:14 GMT+02:00 Kai Tietz :
>>
>>> 2015-07-27 18:51 GMT+02:00 Jason Merrill :
I've trimmed this to the previously mentioned issues that still need to
be
addressed; I'll do another full review after these are dealt with.
>>>
>>>
>>> Thanks for doing this summary of missing parts of prior review.
>>>
On 06/13/2015 12:15 AM, Jason Merrill wrote:
>
>
> On 06/12/2015 12:11 PM, Kai Tietz wrote:
@@ -1052,6 +1054,9 @@ adjust_temp_type (tree type, tree temp)
{
if (TREE_TYPE (temp) == type)
return temp;
+ STRIP_NOPS (temp);
+ if (TREE_TYPE (temp) == type)
+return temp;
@@ -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.
The change to adjust_temp_type seems to be no more necessary (just
doing tests on it).
@@ -1088,7 +1093,10 @@ cxx_bind_parameters_in_call (const
constexpr_ctx
*ctx, tree t,
&& is_dummy_object (x))
{
x = ctx->object;
- x = cp_build_addr_expr (x, tf_warning_or_error);
+ if (x)
+ x = cp_build_addr_expr (x, tf_warning_or_error);
+ else
+ x = get_nth_callarg (t, i);
>>>
>>>
>>>
>>> This still should not be necessary.
Replaced the x = get_nth_callarg (t,i); by a gcc_unreachable ();,
just to be sure we hit issue, if occures.
>>
>>
>> Yeah, most likely. But I got initially here some issues, so I don't
>> see that this code would worsen things.
>
>
>
> If this code path is hit, that means something has broken my design,
> and
> I don't want to just paper over that. Please revert this change.
>
>
> ^
>
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).
>>
>>
>> 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.
>
@@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const
constexpr_ctx
*ctx, tree t,
case CONVERT_EXPR:
case VIEW_CONVERT_EXPR:
case NOP_EXPR:
+case UNARY_PLUS_EXPR:
{
+ enum tree_code tcode = TREE_CODE (t);
tree oldop = TREE_OPERAND (t, 0);
+
+ if (tcode == NOP_EXPR && TREE_TYPE (t) == TREE_TYPE (oldop)
&&
TREE_OVERFLOW_P (oldop))
+ {
+ if (!ctx->quiet)
+ permerror (input_location, "overflow in constant
expression");
+ /* If we're being permissive (and are in an enforcing
+ context), ignore the overflow. */
+ if (!flag_permissive)
+ *overflow_p = true;
+ *non_constant_p = true;
+
+ return t;
+ }
tree op = cxx_eval_constant_expression (ctx, oldop,
>>>
>>>
>>>
>>>
Re: C++ delayed folding branch review
On 07/28/2015 04:10 PM, Kai Tietz wrote:
2015-07-28 1:14 GMT+02:00 Kai Tietz :
2015-07-27 18:51 GMT+02:00 Jason Merrill :
I've trimmed this to the previously mentioned issues that still need to be
addressed; I'll do another full review after these are dealt with.
Thanks for doing this summary of missing parts of prior review.
On 06/13/2015 12:15 AM, Jason Merrill wrote:
On 06/12/2015 12:11 PM, Kai Tietz wrote:
@@ -1052,6 +1054,9 @@ adjust_temp_type (tree type, tree temp)
{
if (TREE_TYPE (temp) == type)
return temp;
+ STRIP_NOPS (temp);
+ if (TREE_TYPE (temp) == type)
+return temp;
@@ -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);
Within the constexpr code we should be folding away NOPs as they are
generated, they shouldn't live this long.
Well, we might see them on overflows ...
We shouldn't within the constexpr code. NOPs for expressions that are
non-constant due to overflow are added in
cxx_eval_outermost_constant_expr, so we shouldn't see them in the middle
of constexpr evaluation.
^
@@ -1088,7 +1093,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx
*ctx, tree t,
&& is_dummy_object (x))
{
x = ctx->object;
- x = cp_build_addr_expr (x, tf_warning_or_error);
+ if (x)
+ x = cp_build_addr_expr (x, tf_warning_or_error);
+ else
+ x = get_nth_callarg (t, i);
This still should not be necessary.
Yeah, most likely. But I got initially here some issues, so I don't
see that this code would worsen things.
If this code path is hit, that means something has broken my design, and
I don't want to just paper over that. Please revert this change.
^
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?
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.
^
@@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const
constexpr_ctx
*ctx, tree t,
case CONVERT_EXPR:
case VIEW_CONVERT_EXPR:
case NOP_EXPR:
+case UNARY_PLUS_EXPR:
{
+ enum tree_code tcode = TREE_CODE (t);
tree oldop = TREE_OPERAND (t, 0);
+
+ if (tcode == NOP_EXPR && TREE_TYPE (t) == TREE_TYPE (oldop) &&
TREE_OVERFLOW_P (oldop))
+ {
+ if (!ctx->quiet)
+ permerror (input_location, "overflow in constant
expression");
+ /* If we're being permissive (and are in an enforcing
+ context), ignore the overflow. */
+ if (!flag_permissive)
+ *overflow_p = true;
+ *non_constant_p = true;
+
+ return t;
+ }
tree op = cxx_eval_constant_expression (ctx, oldop,
Why doesn't the call to cxx_eval_constant_expression at the bottom here
handle oldop having TREE_OVERFLOW set?
I just handled the case that we see here a wrapping NOP_EXPR around an
overflow. As this isn't handled by cxx_eval_constant_expression.
How does it need to be handled? A NOP_EXPR wrapped around an overflow
is there to indicated that the expression is non-constant, and it can't
be simplified any farther.
Please give an example of what was going wrong.
^
@@ -565,6 +571,23 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p)
switch (code)
{
+case SIZEOF_EXPR:
+ if (SIZEOF_EXPR_TYPE_P (*expr_p))
+ *expr_p = cxx_sizeof_or_alignof_type (TREE_TYPE (TREE_OPERAND
(*expr_p,
+
0)),
+ SIZEOF_EXPR, false);
+ else if (TYPE_P (TREE_OPERAND (*expr_p, 0)))
+ *expr_p = cxx_sizeof_or_alignof_type (TREE_OPERAND (*expr_p,
0),
+ SIZEOF_EXPR, false);
+ else
+ *expr_p = cxx_sizeof_or_alignof_expr (TREE_OPERAND (*expr_p,
0),
+ SIZEOF_EXPR, false);
+ if (*expr_p == error_mark_node)
+ *expr_p = size_one_node;
+
+ *expr_p = maybe_constant_value (*expr_p);
+ ret = GS_OK;
+ break;
Why are these surviving until gimplification time?
This might be still necessary. I will retest, when bootstrap works.
As we now added SIZEOF_EXPR folding to cp_fold, and if we catch all
expressions a sizeof can occure, this shouldn't be necessary anymore.
AFAIR I saw here some issues about initialzation for global-variables,
which we
Re: C++ delayed folding branch review
2015-07-28 1:14 GMT+02:00 Kai Tietz :
> 2015-07-27 18:51 GMT+02:00 Jason Merrill :
>> I've trimmed this to the previously mentioned issues that still need to be
>> addressed; I'll do another full review after these are dealt with.
>
> Thanks for doing this summary of missing parts of prior review.
>
>> On 06/13/2015 12:15 AM, Jason Merrill wrote:
>>>
>>> On 06/12/2015 12:11 PM, Kai Tietz wrote:
>>
>> @@ -1052,6 +1054,9 @@ adjust_temp_type (tree type, tree temp)
>> {
>> if (TREE_TYPE (temp) == type)
>> return temp;
>> + STRIP_NOPS (temp);
>> + if (TREE_TYPE (temp) == type)
>> +return temp;
>> @@ -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);
>
>
> Within the constexpr code we should be folding away NOPs as they are
> generated, they shouldn't live this long.
Well, we might see them on overflows ...
>>>
>>>
>>> We shouldn't within the constexpr code. NOPs for expressions that are
>>> non-constant due to overflow are added in
>>> cxx_eval_outermost_constant_expr, so we shouldn't see them in the middle
>>> of constexpr evaluation.
>>>
>> @@ -1088,7 +1093,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx
>> *ctx, tree t,
>>&& is_dummy_object (x))
>> {
>>x = ctx->object;
>> - x = cp_build_addr_expr (x, tf_warning_or_error);
>> + if (x)
>> + x = cp_build_addr_expr (x, tf_warning_or_error);
>> + else
>> + x = get_nth_callarg (t, i);
>
>
> This still should not be necessary.
Yeah, most likely. But I got initially here some issues, so I don't
see that this code would worsen things.
>>>
>>>
>>> If this code path is hit, that means something has broken my design, and
>>> I don't want to just paper over that. Please revert this change.
>>>
>> 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?
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.
>>>
>> @@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const
>> constexpr_ctx
>> *ctx, tree t,
>> case CONVERT_EXPR:
>> case VIEW_CONVERT_EXPR:
>> case NOP_EXPR:
>> +case UNARY_PLUS_EXPR:
>> {
>> + enum tree_code tcode = TREE_CODE (t);
>> tree oldop = TREE_OPERAND (t, 0);
>> +
>> + if (tcode == NOP_EXPR && TREE_TYPE (t) == TREE_TYPE (oldop) &&
>> TREE_OVERFLOW_P (oldop))
>> + {
>> + if (!ctx->quiet)
>> + permerror (input_location, "overflow in constant
>> expression");
>> + /* If we're being permissive (and are in an enforcing
>> + context), ignore the overflow. */
>> + if (!flag_permissive)
>> + *overflow_p = true;
>> + *non_constant_p = true;
>> +
>> + return t;
>> + }
>> tree op = cxx_eval_constant_expression (ctx, oldop,
>
>
> Why doesn't the call to cxx_eval_constant_expression at the bottom here
> handle oldop having TREE_OVERFLOW set?
I just handled the case that we see here a wrapping NOP_EXPR around an
overflow. As this isn't handled by cxx_eval_constant_expression.
>>>
>>>
>>> How does it need to be handled? A NOP_EXPR wrapped around an overflow
>>> is there to indicated that the expression is non-constant, and it can't
>>> be simplified any farther.
>>>
>>> Please give an example of what was going wrong.
>>>
>> @@ -565,6 +571,23 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
>> gimple_seq *post_p)
>>
>> switch (code)
>> {
>> +case SIZEOF_EXPR:
>> + if (SIZEOF_EXPR_TYPE_P (*expr_p))
>> + *expr_p = cxx_sizeof_or_alignof_type (TREE_TYPE (TREE_OPERAND
>> (*expr_p,
>> +
>> 0)),
>> + SIZEOF_EXPR, false);
>> + else if (TYPE_P (TREE_OPERAND (*expr_p, 0)))
>> + *expr_p = cxx_sizeof_or_alignof_type (TREE_OPERAND (*expr_p,
>> 0),
>> +
Re: C++ delayed folding branch review
2015-07-27 18:51 GMT+02:00 Jason Merrill :
> I've trimmed this to the previously mentioned issues that still need to be
> addressed; I'll do another full review after these are dealt with.
Thanks for doing this summary of missing parts of prior review.
> On 06/13/2015 12:15 AM, Jason Merrill wrote:
>>
>> On 06/12/2015 12:11 PM, Kai Tietz wrote:
>
> @@ -1052,6 +1054,9 @@ adjust_temp_type (tree type, tree temp)
> {
> if (TREE_TYPE (temp) == type)
> return temp;
> + STRIP_NOPS (temp);
> + if (TREE_TYPE (temp) == type)
> +return temp;
> @@ -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);
Within the constexpr code we should be folding away NOPs as they are
generated, they shouldn't live this long.
>>>
>>>
>>> Well, we might see them on overflows ...
>>
>>
>> We shouldn't within the constexpr code. NOPs for expressions that are
>> non-constant due to overflow are added in
>> cxx_eval_outermost_constant_expr, so we shouldn't see them in the middle
>> of constexpr evaluation.
>>
> @@ -1088,7 +1093,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx
> *ctx, tree t,
>&& is_dummy_object (x))
> {
>x = ctx->object;
> - x = cp_build_addr_expr (x, tf_warning_or_error);
> + if (x)
> + x = cp_build_addr_expr (x, tf_warning_or_error);
> + else
> + x = get_nth_callarg (t, i);
This still should not be necessary.
>>>
>>>
>>> Yeah, most likely. But I got initially here some issues, so I don't
>>> see that this code would worsen things.
>>
>>
>> If this code path is hit, that means something has broken my design, and
>> I don't want to just paper over that. Please revert this change.
>>
> 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?
>>>
>>>
>>> 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.
>>
> @@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const
> constexpr_ctx
> *ctx, tree t,
> case CONVERT_EXPR:
> case VIEW_CONVERT_EXPR:
> case NOP_EXPR:
> +case UNARY_PLUS_EXPR:
> {
> + enum tree_code tcode = TREE_CODE (t);
> tree oldop = TREE_OPERAND (t, 0);
> +
> + if (tcode == NOP_EXPR && TREE_TYPE (t) == TREE_TYPE (oldop) &&
> TREE_OVERFLOW_P (oldop))
> + {
> + if (!ctx->quiet)
> + permerror (input_location, "overflow in constant
> expression");
> + /* If we're being permissive (and are in an enforcing
> + context), ignore the overflow. */
> + if (!flag_permissive)
> + *overflow_p = true;
> + *non_constant_p = true;
> +
> + return t;
> + }
> tree op = cxx_eval_constant_expression (ctx, oldop,
Why doesn't the call to cxx_eval_constant_expression at the bottom here
handle oldop having TREE_OVERFLOW set?
>>>
>>>
>>> I just handled the case that we see here a wrapping NOP_EXPR around an
>>> overflow. As this isn't handled by cxx_eval_constant_expression.
>>
>>
>> How does it need to be handled? A NOP_EXPR wrapped around an overflow
>> is there to indicated that the expression is non-constant, and it can't
>> be simplified any farther.
>>
>> Please give an example of what was going wrong.
>>
> @@ -565,6 +571,23 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
> gimple_seq *post_p)
>
> switch (code)
> {
> +case SIZEOF_EXPR:
> + if (SIZEOF_EXPR_TYPE_P (*expr_p))
> + *expr_p = cxx_sizeof_or_alignof_type (TREE_TYPE (TREE_OPERAND
> (*expr_p,
> +
> 0)),
> + SIZEOF_EXPR, false);
> + else if (TYPE_P (TREE_OPERAND (*expr_p, 0)))
> + *expr_p = cxx_sizeof_or_alignof_type (TREE_OPERAND (*expr_p,
> 0),
> + SIZEOF_EXPR, false);
> + else
> + *expr_p = cxx_sizeof_or_alignof_expr (TREE_OPERAND (*expr_p,
> 0),
> +
Re: C++ delayed folding branch review
I've trimmed this to the previously mentioned issues that still need to
be addressed; I'll do another full review after these are dealt with.
On 06/13/2015 12:15 AM, Jason Merrill wrote:
On 06/12/2015 12:11 PM, Kai Tietz wrote:
@@ -1052,6 +1054,9 @@ adjust_temp_type (tree type, tree temp)
{
if (TREE_TYPE (temp) == type)
return temp;
+ STRIP_NOPS (temp);
+ if (TREE_TYPE (temp) == type)
+return temp;
@@ -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);
Within the constexpr code we should be folding away NOPs as they are
generated, they shouldn't live this long.
Well, we might see them on overflows ...
We shouldn't within the constexpr code. NOPs for expressions that are
non-constant due to overflow are added in
cxx_eval_outermost_constant_expr, so we shouldn't see them in the middle
of constexpr evaluation.
@@ -1088,7 +1093,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx
*ctx, tree t,
&& is_dummy_object (x))
{
x = ctx->object;
- x = cp_build_addr_expr (x, tf_warning_or_error);
+ if (x)
+ x = cp_build_addr_expr (x, tf_warning_or_error);
+ else
+ x = get_nth_callarg (t, i);
This still should not be necessary.
Yeah, most likely. But I got initially here some issues, so I don't
see that this code would worsen things.
If this code path is hit, that means something has broken my design, and
I don't want to just paper over that. Please revert this change.
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?
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.
@@ -3391,8 +3431,23 @@ cxx_eval_constant_expression (const
constexpr_ctx
*ctx, tree t,
case CONVERT_EXPR:
case VIEW_CONVERT_EXPR:
case NOP_EXPR:
+case UNARY_PLUS_EXPR:
{
+ enum tree_code tcode = TREE_CODE (t);
tree oldop = TREE_OPERAND (t, 0);
+
+ if (tcode == NOP_EXPR && TREE_TYPE (t) == TREE_TYPE (oldop) &&
TREE_OVERFLOW_P (oldop))
+ {
+ if (!ctx->quiet)
+ permerror (input_location, "overflow in constant
expression");
+ /* If we're being permissive (and are in an enforcing
+ context), ignore the overflow. */
+ if (!flag_permissive)
+ *overflow_p = true;
+ *non_constant_p = true;
+
+ return t;
+ }
tree op = cxx_eval_constant_expression (ctx, oldop,
Why doesn't the call to cxx_eval_constant_expression at the bottom here
handle oldop having TREE_OVERFLOW set?
I just handled the case that we see here a wrapping NOP_EXPR around an
overflow. As this isn't handled by cxx_eval_constant_expression.
How does it need to be handled? A NOP_EXPR wrapped around an overflow
is there to indicated that the expression is non-constant, and it can't
be simplified any farther.
Please give an example of what was going wrong.
@@ -565,6 +571,23 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p,
gimple_seq *post_p)
switch (code)
{
+case SIZEOF_EXPR:
+ if (SIZEOF_EXPR_TYPE_P (*expr_p))
+ *expr_p = cxx_sizeof_or_alignof_type (TREE_TYPE (TREE_OPERAND
(*expr_p,
+
0)),
+ SIZEOF_EXPR, false);
+ else if (TYPE_P (TREE_OPERAND (*expr_p, 0)))
+ *expr_p = cxx_sizeof_or_alignof_type (TREE_OPERAND (*expr_p,
0),
+ SIZEOF_EXPR, false);
+ else
+ *expr_p = cxx_sizeof_or_alignof_expr (TREE_OPERAND (*expr_p,
0),
+ SIZEOF_EXPR, false);
+ if (*expr_p == error_mark_node)
+ *expr_p = size_one_node;
+
+ *expr_p = maybe_constant_value (*expr_p);
+ ret = GS_OK;
+ break;
Why are these surviving until gimplification time?
This might be still necessary. I will retest, when bootstrap works.
As we now added SIZEOF_EXPR folding to cp_fold, and if we catch all
expressions a sizeof can occure, this shouldn't be necessary anymore.
AFAIR I saw here some issues about initialzation for global-variables,
which weren't caught.
Hmm, I wonder why you would see issues with global initializers that
aren't seen on trunk? In any case, if the issue is with global
initializers, they should be handled sooner, not here.
@@ -608,9 +608,13 @@ cp_fold_conver
Re: C++ delayed folding branch review
On 06/12/2015 12:11 PM, Kai Tietz wrote:
@@ -589,9 +589,9 @@ null_member_pointer_value_p (tree t)
return false;
else if (TYPE_PTRMEMFUNC_P (type))
return (TREE_CODE (t) == CONSTRUCTOR
- && integer_zerop (CONSTRUCTOR_ELT (t, 0)->value));
+ && integer_zerop (fold (CONSTRUCTOR_ELT (t, 0)->value)));
else if (TYPE_PTRDATAMEM_P (type))
-return integer_all_onesp (t);
+return integer_all_onesp (fold (t));
Again, calling fold here is wrong; it doesn't handle constexpr, and we
should have folded before we got here.
Agreed. I will commit change for this.
Nevertheless CONSTRUCTOR_ELT's value might still be prefixed by nops due
possible overflows, or by negative sign/invert/etc.
It shouldn't in any calls to this function; the argument to this
function should have already been folded.
@@ -5090,9 +5090,9 @@ build_conditional_expr_1 (location_t loc, tree arg1,
tree arg2, tree arg3,
valid_operands:
result = build3 (COND_EXPR, result_type, arg1, arg2, arg3);
- if (!cp_unevaluated_operand)
+ if (!cp_unevaluated_operand && !processing_template_decl)
/* Avoid folding within decltype (c++/42013) and noexcept. */
-result = fold_if_not_in_template (result);
+result = fold (result);
This seems related to your status report note:
Additionally addressed issue about cond_expr, as we want to fold cases with
a constant-condition. Here we need to use "fold_to_constant" so that we
just fold things to constant-value, if possible and otherwise don't modify
anything.
But why do you say we want to fold cases with a constant condition? We
certainly want to avoid warning about the dead branch in that case, but
it would be better if we can do that folding only in the warning code.
Issue is that we otherwise detect in conditions that expressions aren't
constant due never-executed-code-path.
How so? The code for determining whether an expression is constant
should do the folding.
I think the way to avoid warnings about dead code paths is to do the
folding in cp_parser_question_colon_clause and in tsubst_copy_and_build,
case COND_EXPR.
The diagnostics we can deal differently, but this was actually the reason for
doing this. I can remove this here, but we still need a place to avoid ill
detection of constexpr (or invalid code) on dead code-branch. Eg. (1 ? 0/0 :
1) etc
@@ -7382,8 +7383,13 @@ build_over_call (struct z_candidate *cand, int
flags, tsu
bst_flags_t complain)
gcc_assert (j <= nargs);
nargs = j;
+ {
+tree *fargs = (!nargs ? argarray : (tree *) alloca (nargs * sizeof
(tree)))
;
+for (j = 0; j < nargs; j++)
+ fargs[j] = fold_non_dependent_expr (argarray[j]);
No change needed here, but I notice that fold_non_dependent_expr is
still using maybe_constant_value; it should probably use cp_fully_fold
instead.
Hmm, maybe we should limit this folding on constants. So cp_fold_to_constant
()?
This folding is just for diagnostics, so I think cp_fully_fold is the
right choice.
@@ -1052,6 +1054,9 @@ adjust_temp_type (tree type, tree temp)
{
if (TREE_TYPE (temp) == type)
return temp;
+ STRIP_NOPS (temp);
+ if (TREE_TYPE (temp) == type)
+return temp;
@@ -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);
Within the constexpr code we should be folding away NOPs as they are
generated, they shouldn't live this long.
Well, we might see them on overflows ...
We shouldn't within the constexpr code. NOPs for expressions that are
non-constant due to overflow are added in
cxx_eval_outermost_constant_expr, so we shouldn't see them in the middle
of constexpr evaluation.
@@ -1088,7 +1093,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx
*ctx, tree t,
&& is_dummy_object (x))
{
x = ctx->object;
- x = cp_build_addr_expr (x, tf_warning_or_error);
+ if (x)
+ x = cp_build_addr_expr (x, tf_warning_or_error);
+ else
+ x = get_nth_callarg (t, i);
This still should not be necessary.
Yeah, most likely. But I got initially here some issues, so I don't see that
this code would worsen things.
If this code path is hit, that means something has broken my design, and
I don't want to just paper over that. Please revert this change.
@@ -1576,13 +1586,15 @@ cxx_eval_unary_expression (const constexpr_ctx
*ctx, tre
e t,
enum tree_code code = TREE_CODE (t);
tree type = TREE_TYPE (t);
r = fold_unary_loc (loc, code, type, arg);
- if (r == NULL_TREE)
+ if (r == NULL_TREE || !CONSTANT_CLASS_P (r))
{
if (arg == orig_arg)
r = t;
else
r = build1_loc (loc, code, type, arg);
}
+ else
+r = unify_constant (ctx, r, overflow_p);
This still should not be necessary.
Well, I just wanted to make sure that if arg i
Re: C++ delayed folding branch review
Hello Jason,
Thanks for the review. I addressed a lot of your comments directly on
svn-branch. See revision r224439.
- Ursprüngliche Mail -
> Generally, it seems like most of my comments from April haven't been
> addressed yet.
Yes, most of them.
> > @@ -3023,13 +3023,14 @@ conversion_warning (location_t loc, tree type, tree
> > expr
>
> Instead of adding folds here, let's make sure that the argument we pass
> (e.g. from cp_convert_and_check to warnings_for_convert_and_check) is
> fully folded.
I looked for dependencies, and address this.
> > @@ -589,9 +589,9 @@ null_member_pointer_value_p (tree t)
> > return false;
> >else if (TYPE_PTRMEMFUNC_P (type))
> > return (TREE_CODE (t) == CONSTRUCTOR
> > - && integer_zerop (CONSTRUCTOR_ELT (t, 0)->value));
> > + && integer_zerop (fold (CONSTRUCTOR_ELT (t, 0)->value)));
> >else if (TYPE_PTRDATAMEM_P (type))
> > -return integer_all_onesp (t);
> > +return integer_all_onesp (fold (t));
>
> Again, calling fold here is wrong; it doesn't handle constexpr, and we
> should have folded before we got here.
Agreed. I will commit change for this.
Nevertheless CONSTRUCTOR_ELT's value might still be prefixed by nops due
possible overflows, or by negative sign/invert/etc. This is caused by
non-constant-folding of values. I removed for now those folds, as I agree we
should address this at place of value-assignment. Nevertheless we still have
this issue
> > @@ -5090,9 +5090,9 @@ build_conditional_expr_1 (location_t loc, tree arg1,
> > tree arg2, tree arg3,
> >
> > valid_operands:
> >result = build3 (COND_EXPR, result_type, arg1, arg2, arg3);
> > - if (!cp_unevaluated_operand)
> > + if (!cp_unevaluated_operand && !processing_template_decl)
> > /* Avoid folding within decltype (c++/42013) and noexcept. */
> > -result = fold_if_not_in_template (result);
> > +result = fold (result);
>
> This seems related to your status report note:
>
> > Additionally addressed issue about cond_expr, as we want to fold cases with
> > a constant-condition. Here we need to use "fold_to_constant" so that we
> > just fold things to constant-value, if possible and otherwise don't modify
> > anything.
>
> But why do you say we want to fold cases with a constant condition? We
> certainly want to avoid warning about the dead branch in that case, but
> it would be better if we can do that folding only in the warning code.
Issue is that we otherwise detect in conditions that expressions aren't
constant due never-executed-code-path. The diagnostics we can deal
differently, but this was actually the reason for doing this. I can remove
this here, but we still need a place to avoid ill detection of constexpr (or
invalid code) on dead code-branch. Eg. (1 ? 0/0 : 1) etc
> > @@ -5628,8 +5628,8 @@ build_new_op_1 (location_t loc, enum tree_code code,
> > int f
> > lags, tree arg1,
> > decaying an enumerator to its value. */
> > if (complain & tf_warning)
> > warn_logical_operator (loc, code, boolean_type_node,
> > - code_orig_arg1, arg1,
> > - code_orig_arg2, arg2);
> > + code_orig_arg1, fold (arg1),
> > + code_orig_arg2, fold (arg2));
> >
> > arg2 = convert_like (conv, arg2, complain);
> > }
> > @@ -5666,7 +5666,8 @@ build_new_op_1 (location_t loc, enum tree_code code,
> > int f
> > lags, tree arg1,
> > case TRUTH_AND_EXPR:
> > case TRUTH_OR_EXPR:
> >warn_logical_operator (loc, code, boolean_type_node,
> > -code_orig_arg1, arg1, code_orig_arg2, arg2);
> > +code_orig_arg1, fold (arg1),
> > +code_orig_arg2, fold (arg2));
> >/* Fall through. */
> > case GT_EXPR:
> > case LT_EXPR:
> > @@ -5676,7 +5677,7 @@ build_new_op_1 (location_t loc, enum tree_code code,
> > int f
> > lags, tree arg1,
> > case NE_EXPR:
> >if ((code_orig_arg1 == BOOLEAN_TYPE)
> > ^ (code_orig_arg2 == BOOLEAN_TYPE))
> > - maybe_warn_bool_compare (loc, code, arg1, arg2);
> > + maybe_warn_bool_compare (loc, code, fold (arg1), fold (arg2));
> >/* Fall through. */
> > case PLUS_EXPR:
> > case MINUS_EXPR:
>
> I still think these fold calls should be cp_fully_fold.
Ok, modified here use of fold to cp_fully_fold.
>
> > @@ -7382,8 +7383,13 @@ build_over_call (struct z_candidate *cand, int
> > flags, tsu
> > bst_flags_t complain)
> >
> >gcc_assert (j <= nargs);
> >nargs = j;
> > + {
> > +tree *fargs = (!nargs ? argarray : (tree *) alloca (nargs * sizeof
> > (tree)))
> > ;
> > +for (j = 0; j < nargs; j++)
> > + fargs[j] = fold_non_dependent_expr (argarray[j]);
>
> No change needed here, but I notice that fold_non_dependent_expr is
>
C++ delayed folding branch review
Generally, it seems like most of my comments from April haven't been
addressed yet.
@@ -3023,13 +3023,14 @@ conversion_warning (location_t loc, tree type, tree expr
Instead of adding folds here, let's make sure that the argument we pass
(e.g. from cp_convert_and_check to warnings_for_convert_and_check) is
fully folded.
@@ -589,9 +589,9 @@ null_member_pointer_value_p (tree t)
return false;
else if (TYPE_PTRMEMFUNC_P (type))
return (TREE_CODE (t) == CONSTRUCTOR
- && integer_zerop (CONSTRUCTOR_ELT (t, 0)->value));
+ && integer_zerop (fold (CONSTRUCTOR_ELT (t, 0)->value)));
else if (TYPE_PTRDATAMEM_P (type))
-return integer_all_onesp (t);
+return integer_all_onesp (fold (t));
Again, calling fold here is wrong; it doesn't handle constexpr, and we
should have folded before we got here.
@@ -5090,9 +5090,9 @@ build_conditional_expr_1 (location_t loc, tree arg1, tree
arg2, tree arg3,
valid_operands:
result = build3 (COND_EXPR, result_type, arg1, arg2, arg3);
- if (!cp_unevaluated_operand)
+ if (!cp_unevaluated_operand && !processing_template_decl)
/* Avoid folding within decltype (c++/42013) and noexcept. */
-result = fold_if_not_in_template (result);
+result = fold (result);
This seems related to your status report note:
Additionally addressed issue about cond_expr, as we want to fold cases with a
constant-condition. Here we need to use "fold_to_constant" so that we just
fold things to constant-value, if possible and otherwise don't modify anything.
But why do you say we want to fold cases with a constant condition? We
certainly want to avoid warning about the dead branch in that case, but
it would be better if we can do that folding only in the warning code.
@@ -5628,8 +5628,8 @@ build_new_op_1 (location_t loc, enum tree_code code, int f
lags, tree arg1,
decaying an enumerator to its value. */
if (complain & tf_warning)
warn_logical_operator (loc, code, boolean_type_node,
- code_orig_arg1, arg1,
- code_orig_arg2, arg2);
+ code_orig_arg1, fold (arg1),
+ code_orig_arg2, fold (arg2));
arg2 = convert_like (conv, arg2, complain);
}
@@ -5666,7 +5666,8 @@ build_new_op_1 (location_t loc, enum tree_code code, int f
lags, tree arg1,
case TRUTH_AND_EXPR:
case TRUTH_OR_EXPR:
warn_logical_operator (loc, code, boolean_type_node,
-code_orig_arg1, arg1, code_orig_arg2, arg2);
+code_orig_arg1, fold (arg1),
+code_orig_arg2, fold (arg2));
/* Fall through. */
case GT_EXPR:
case LT_EXPR:
@@ -5676,7 +5677,7 @@ build_new_op_1 (location_t loc, enum tree_code code, int f
lags, tree arg1,
case NE_EXPR:
if ((code_orig_arg1 == BOOLEAN_TYPE)
^ (code_orig_arg2 == BOOLEAN_TYPE))
- maybe_warn_bool_compare (loc, code, arg1, arg2);
+ maybe_warn_bool_compare (loc, code, fold (arg1), fold (arg2));
/* Fall through. */
case PLUS_EXPR:
case MINUS_EXPR:
I still think these fold calls should be cp_fully_fold.
@@ -7382,8 +7383,13 @@ build_over_call (struct z_candidate *cand, int flags, tsu
bst_flags_t complain)
gcc_assert (j <= nargs);
nargs = j;
+ {
+tree *fargs = (!nargs ? argarray : (tree *) alloca (nargs * sizeof (tree)))
;
+for (j = 0; j < nargs; j++)
+ fargs[j] = fold_non_dependent_expr (argarray[j]);
No change needed here, but I notice that fold_non_dependent_expr is
still using maybe_constant_value; it should probably use cp_fully_fold
instead.
@@ -1052,6 +1054,9 @@ adjust_temp_type (tree type, tree temp)
{
if (TREE_TYPE (temp) == type)
return temp;
+ STRIP_NOPS (temp);
+ if (TREE_TYPE (temp) == type)
+return temp;
@@ -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);
Within the constexpr code we should be folding away NOPs as they are
generated, they shouldn't live this long.
@@ -1088,7 +1093,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx,
tree t,
&& is_dummy_object (x))
{
x = ctx->object;
- x = cp_build_addr_expr (x, tf_warning_or_error);
+ if (x)
+ x = cp_build_addr_expr (x, tf_warning_or_error);
+ else
+ x = get_nth_callarg (t, i);
This still should not be necessary.
@@ -1576,13 +1586,15 @@ cxx_eval_unary_expression (const constexpr_ctx *ctx, tre
e t,
enum tree_code code = TREE_CODE (t);
tree type = TREE_TYPE (t);
r = fold_unary_loc (loc, code, type, arg);
- if (r == NULL_TREE)
+ if (r == NULL_TREE || !CONSTANT_CLASS_P (r))
{
if (arg == orig_arg
Re: C++ delayed folding branch review
On 04/28/2015 08:06 AM, Kai Tietz wrote: 2015-04-24 20:25 GMT+02:00 Jason Merrill : So for warning folding, I think the caching approach we were talking about in the call today is the way to go. Ok. Just a question about where to hook up the hash-variable. What would be a good place to create (well this we could do lazy too) it, and of more importance where to destroy the hash? We could destroy it after genericize-pass? The important thing is to avoid keeping exprs in the hash table that have already been ggc_collected. So we want to destroy the hash table at least after every top-level declaration, but destroying it more frequently would be fine too if memory bloat seems like an issue. Certainly a constant value from maybe_constant_value should not have a useless type conversion wrapped around it, as that makes it appear non-constant. Well, beside an overflow was seen. Right. The interesting point is here to find the proper place to do for constructor-elements proper folding, or at least constant-value folding. cxx_eval_bare_aggregate should already be doing constant-value folding of constructor elements. I think that's exactly what I was saying above: "we can't just use maybe_constant_value because that only folds C++ constant-expressions, and we want to fold more things than that." maybe_constant_value folds constants, too. That was actually the reason to use it. Nevertheless I admit that we could call instead a ..fully_fold here too. Call it where? One point I see here about creating a function using maybe_constant_value + cp_fold is that maybe_constant_value is something we can call safely in all cases. Right, we would still want the current maybe_constant_value for the few situations where constant expressions affect language semantics but are not required, such as initializers and array bounds. I'm suggesting that we should add maybe_constant_value to cp_fold, not the other way around. My point is that cp_fold should be a superset of maybe_constant_value, to fix bugs like 53792. And the easiest way to get that would seem to be by calling maybe_constant_value from cp_fold. Agreed, that bugs like PR 53792 could be solved that way. Nevertheless they aren't directly linked to the delayed-folding problem, are they? We could deal with those cases also in genericize-pass lately, as we want to catch here just a missed optimization, aren't we? They aren't due to premature folding, but they are due to the lack of delayed folding. If we're going to fully fold expressions at genericize time (and at other times for warnings), that needs to include folding calls to constexpr functions, or it isn't "full" folding. We want to handle them together rather than separately because they can interact: simplifying one expression with fold may turn it into a C++ constant-expression (which is why we have premature-folding bugs), and simplifying a constant-expression may expose more opportunities for fold. And we only want a single hash table. OK, that makes sense. Say, a function called like "fold" that only folds conversions (and NEGATE_EXPR) of constants. It might make sense to do that and otherwise continue to delay folding of conversions. In that context I guess this change makes sense. Fine, any preferences about place for this function? I suggest the name 'fold_cst' for it. Sounds good. Yes, but we already called maybe_constant_value [in compute_array_index_type]. Calling it again shouldn't make any difference. We just call it in non-dependent tree, If the expression is dependent, maybe_constant_value does nothing. and even here just for special cases. Special cases? It looks like it's called if the expression is not a magic C++98 NOP_EXPR with TREE_SIDE_EFFECTS and it is convertible to integral or enumeration type. That seems to cover all cases of constant array bounds. Do you have a testcase that motivates this? - itype = fold (itype); + itype = maybe_constant_value (itype); - itype = variable_size (fold (newitype)); + itype = variable_size (maybe_constant_value (newitype)); Maybe these should use cp_fully_fold? We could use fully_fold, but we would also modify none-constant expressions by this. Do we actually want that here? For the first one, I actually think a plain fold is enough, or perhaps change the cp_build_binary_op to size_binop. Hmm, I doubt that a simple fold would be enough here. As itype is calculated as minus of two converts (and cp_converts aren't automatically folded). Sure, the converts need to be folded as well, but we were just saying that we are going to automatically fold conversion of a constant, right? @@ -13090,6 +13092,8 @@ build_enumerator (tree name, tree value, tree enumtype, location_t loc) + if (value) +value = maybe_constant_value (value); As above, calling the constexpr code twice shouldn't make a difference. Well, issue is tha
Re: C++ delayed folding branch review
2015-04-24 20:25 GMT+02:00 Jason Merrill :
> On 04/24/2015 09:46 AM, Kai Tietz wrote:
>>
>> Sure, we can use here instead *_fully_fold, but for what costs? In
>> general we need to deal here a simple one-level fold for simplifying
>> constant-values, and/or removing useless type-conversions.
>
>
> Well, here you're doing a two-level fold. And in general fold relies on
> subexpressions being appropriately folded. So for warning folding, I think
> the caching approach we were talking about in the call today is the way to
> go.
Ok. Just a question about where to hook up the hash-variable. What
would be a good place to create (well this we could do lazy too) it,
and of more importance where to destroy the hash? We could destroy it
after genericize-pass?
@@ -597,9 +597,9 @@ null_member_pointer_value_p (tree t)
return false;
else if (TYPE_PTRMEMFUNC_P (type))
return (TREE_CODE (t) == CONSTRUCTOR
- && integer_zerop (CONSTRUCTOR_ELT (t, 0)->value));
+ && integer_zerop (fold (CONSTRUCTOR_ELT (t, 0)->value)));
else if (TYPE_PTRDATAMEM_P (type))
-return integer_all_onesp (t);
+return integer_all_onesp (fold (t));
>>>
>>>
>>>
>>> Calling fold here is wrong; it doesn't handle constexpr, and we should
>>> have
>>> folded before we got here.
>>
>>
>> s.o. we need to make sure constant-values get rid of useless
>> types-conversions/negates/etc ...
>
>
> Certainly a constant value from maybe_constant_value should not have a
> useless type conversion wrapped around it, as that makes it appear
> non-constant.
Well, beside an overflow was seen. The interesting point is here to
find the proper place to do for constructor-elements proper folding,
or at least constant-value folding. I made here already some tries to
find a proper place for this, but ran by this into troubles that not
in all cases maybe_constant_value was callable (endless recursion).
Additionally it happens in some cases for constructor-elements that
nop-casts are added on assign. At least I ran into that.
In general there is the question if we shouldn't do for constructors
call a ...fully_fold?
@@ -576,7 +576,6 @@ build_simple_base_path (tree expr, tree binfo)
expr = build3 (COMPONENT_REF,
cp_build_qualified_type (type, type_quals),
expr, field, NULL_TREE);
- expr = fold_if_not_in_template (expr);
>>>
>>>
>>> I don't think we need to remove this fold, since it is part of compiler
>>> internals rather than something the user wrote. Really, we should
>>> represent
>>> the base conversion with something like a CONVERT_EXPR and only call this
>>> function when we want to fold it. But that can wait for a later patch.
>>
>>
>> Ok. I remove this fold-case due simply removing
>> fold_if_not_in_template function. So well, we could re-add a call for
>> fold, if not in template.
>
>
> Let's try not checking for being in a template, see if it breaks.
I tested to use here just a fold (), and I ddn't noticed any fallout
by this. So I committed it to branch.
+static tree
+cp_fold (tree x, hash_map *fold_hash)
+{
>>>
>>>
>>>
>>>
>>> I still think we need a hybrid of this and the constexpr code: it isn't
>>> full
>>> folding if we aren't doing constexpr evaluation. But we can't just use
>>> maybe_constant_value because that only folds C++ constant-expressions,
>>> and
>>> we want to fold more things than that. I suppose one simple approach for
>>> now would be to call maybe_constant_value from cp_fold.
>>
>>
>> Well, the functionality of cp_fold and maybe_constant_value (well,
>> actually how constexpr.c works) are different in cases of
>> non-constant results.
>
>
> I think that's exactly what I was saying above: "we can't just use
> maybe_constant_value because that only folds C++ constant-expressions, and
> we want to fold more things than that."
maybe_constant_value folds constants, too. That was actually the
reason to use it. Nevertheless I admit that we could call instead a
..fully_fold here too.
One point I see here about creating a function using
maybe_constant_value + cp_fold is that maybe_constant_value is
something we can call safely in all cases.
> My point is that cp_fold should be a superset of maybe_constant_value, to
> fix bugs like 53792. And the easiest way to get that would seem to be by
> calling maybe_constant_value from cp_fold.
Agreed, that bugs like PR 53792 could be solved that way.
Nevertheless they aren't directly linked to the delayed-folding
problem, are they? We could deal with those cases also in
genericize-pass lately, as we want to catch here just a missed
optimization, aren't we?
@@ -614,9 +614,13 @@ cp_fold_convert (tree type, tree expr)
}
else
{
- conv = fold_convert (type, expr);
+ if (TREE_CODE (expr) == INTEGER_CST)
+conv = fold_convert (type, ex
Re: C++ delayed folding branch review
On 04/24/2015 09:46 AM, Kai Tietz wrote:
Sure, we can use here instead *_fully_fold, but for what costs? In
general we need to deal here a simple one-level fold for simplifying
constant-values, and/or removing useless type-conversions.
Well, here you're doing a two-level fold. And in general fold relies on
subexpressions being appropriately folded. So for warning folding, I
think the caching approach we were talking about in the call today is
the way to go.
@@ -597,9 +597,9 @@ null_member_pointer_value_p (tree t)
return false;
else if (TYPE_PTRMEMFUNC_P (type))
return (TREE_CODE (t) == CONSTRUCTOR
- && integer_zerop (CONSTRUCTOR_ELT (t, 0)->value));
+ && integer_zerop (fold (CONSTRUCTOR_ELT (t, 0)->value)));
else if (TYPE_PTRDATAMEM_P (type))
-return integer_all_onesp (t);
+return integer_all_onesp (fold (t));
Calling fold here is wrong; it doesn't handle constexpr, and we should have
folded before we got here.
s.o. we need to make sure constant-values get rid of useless
types-conversions/negates/etc ...
Certainly a constant value from maybe_constant_value should not have a
useless type conversion wrapped around it, as that makes it appear
non-constant.
Well, fold_convert isn't necessarily the same as fold (convert ())
within C++, due convert handles special cases fold_convert doesn't.
Ah, true.
@@ -576,7 +576,6 @@ build_simple_base_path (tree expr, tree binfo)
expr = build3 (COMPONENT_REF,
cp_build_qualified_type (type, type_quals),
expr, field, NULL_TREE);
- expr = fold_if_not_in_template (expr);
I don't think we need to remove this fold, since it is part of compiler
internals rather than something the user wrote. Really, we should represent
the base conversion with something like a CONVERT_EXPR and only call this
function when we want to fold it. But that can wait for a later patch.
Ok. I remove this fold-case due simply removing
fold_if_not_in_template function. So well, we could re-add a call for
fold, if not in template.
Let's try not checking for being in a template, see if it breaks.
That said, we should probably just remove this case and the next, as they
are obsolete. I'll remove them on the trunk.
Done.
+static tree
+cp_fold (tree x, hash_map *fold_hash)
+{
I still think we need a hybrid of this and the constexpr code: it isn't full
folding if we aren't doing constexpr evaluation. But we can't just use
maybe_constant_value because that only folds C++ constant-expressions, and
we want to fold more things than that. I suppose one simple approach for
now would be to call maybe_constant_value from cp_fold.
Well, the functionality of cp_fold and maybe_constant_value (well,
actually how constexpr.c works) are different in cases of
non-constant results.
I think that's exactly what I was saying above: "we can't just use
maybe_constant_value because that only folds C++ constant-expressions,
and we want to fold more things than that."
My point is that cp_fold should be a superset of maybe_constant_value,
to fix bugs like 53792. And the easiest way to get that would seem to
be by calling maybe_constant_value from cp_fold.
@@ -614,9 +614,13 @@ cp_fold_convert (tree type, tree expr)
}
else
{
- conv = fold_convert (type, expr);
+ if (TREE_CODE (expr) == INTEGER_CST)
+conv = fold_convert (type, expr);
+ else
+conv = convert (type, expr);
Why? If we're calling cp_fold_convert in a place where we don't want to
fold, we should stop calling it rather than change it.
See, that we want to take care that constant-value is found here.
Otherwise we don't want anything folded. Well, we could introduce
for this a special routine to abstract intention here.
OK, that makes sense. Say, a function called like "fold" that only
folds conversions (and NEGATE_EXPR) of constants. It might make sense
to do that and otherwise continue to delay folding of conversions. In
that context I guess this change makes sense.
@@ -8502,16 +8502,18 @@ compute_array_index_type (tree name, tree size,
tsubst_flags_t complain)
+ /* We need to do fully folding to determine if we have VLA, or not. */
+ tree size_constant = maybe_constant_value (size);
Why is this needed? We already call maybe_constant_value earlier in
compute_array_index_type.
Well, see above. We might have constant-value not simplified. So we
need a way to make sure we simplify in such case, but if it is
none-constant, we don't want an modified expression. So
maybe_constant_value does this ...
Yes, but we already called maybe_constant_value. Calling it again
shouldn't make any difference.
- itype = fold (itype);
+ itype = maybe_constant_value (itype);
- itype = variable_size (fold (newitype));
+ itype = variable_size (maybe_constant_value (newitype));
Maybe these should use cp_fully_f
Re: C++ delayed folding branch review
2015-04-24 6:22 GMT+02:00 Jason Merrill :
>> + expr = fold (expr);
>>/* This may happen, because for LHS op= RHS we preevaluate
>> RHS and create C_MAYBE_CONST_EXPR >, which
>> means we could no longer see the code of the EXPR. */
>>if (TREE_CODE (expr) == C_MAYBE_CONST_EXPR)
>> expr = C_MAYBE_CONST_EXPR_EXPR (expr);
>>if (TREE_CODE (expr) == SAVE_EXPR)
>> -expr = TREE_OPERAND (expr, 0);
>> +expr = fold (TREE_OPERAND (expr, 0));
>
>
> How about moving the first fold after the SAVE_EXPR block, so that we only
> need to call fold once?
I will try. It might be required to fold in front to strip of useless
type-conversions ...
>> +case NEGATE_EXPR:
>> +case BIT_NOT_EXPR:
>> +case CONVERT_EXPR:
>> +case VIEW_CONVERT_EXPR:
>> +case NOP_EXPR:
>> +case FIX_TRUNC_EXPR:
>> + {
>> + tree op1 = TREE_OPERAND (expr, 0);
>> + tree fop1 = fold (op1);
>> + if (fop1 && op1 != fop1)
>> + fop1 = fold_build1_loc (loc, TREE_CODE (expr), TREE_TYPE (expr),
>> + fop1);
well, AFAIR was the idea here to make sure we do constant-value folding ...
>
> Isn't this redundant with the call to fold above? If not, it seems that the
> above call should be to *_fully_fold. I suppose we want an entry point
> defined by both front ends that c-common code can call which does full
> folding of an expression.
Sure, we can use here instead *_fully_fold, but for what costs? In
general we need to deal here a simple one-level fold for simplifying
constant-values, and/or removing useless type-conversions. As convert
doesn't fold them away anymore, it can stay with such NOP_EXPR before
constant-values, which cause us to fail on later value-analyzis. So
sure, we can use *_fully_fold, but actually we don't need a full-fold.
(this applies to other places below too, where you mentioned the same
issue, too).
>> @@ -597,9 +597,9 @@ null_member_pointer_value_p (tree t)
>> return false;
>>else if (TYPE_PTRMEMFUNC_P (type))
>> return (TREE_CODE (t) == CONSTRUCTOR
>> - && integer_zerop (CONSTRUCTOR_ELT (t, 0)->value));
>> + && integer_zerop (fold (CONSTRUCTOR_ELT (t, 0)->value)));
>>else if (TYPE_PTRDATAMEM_P (type))
>> -return integer_all_onesp (t);
>> +return integer_all_onesp (fold (t));
>
>
> Calling fold here is wrong; it doesn't handle constexpr, and we should have
> folded before we got here.
s.o. we need to make sure constant-values get rid of useless
types-conversions/negates/etc ...
>> warn_logical_operator (loc, code, boolean_type_node,
>> - code_orig_arg1, arg1,
>> - code_orig_arg2, arg2);
>> + code_orig_arg1, fold (arg1),
>> + code_orig_arg2, fold (arg2));
>
>
> I think warn_logical_operator should call back into *_fully_fold. Likewise
> for most similar added calls to fold.
Same as above. It isn't necessary for further analysis to perform
fully_fold on arg1/arg2. But of course we can introduce this load
here.
>> @@ -7356,8 +7354,13 @@ build_over_call (struct z_candidate *cand, int
>> flags, tsu
>> bst_flags_t complain)
>>
>>gcc_assert (j <= nargs);
>>nargs = j;
>> + {
>> +tree *fargs = (!nargs ? argarray : (tree *) alloca (nargs * sizeof
>> (tree)))
>> ;
>> +for (j = 0; j < nargs; j++)
>> + fargs[j] = fold_non_dependent_expr (argarray[j]);
>
>
> Similarly, this and build_cxx_call should use cp_fully_fold.
Well, cp_fully_fold wouldn't resolve constexpr ... at least not in completeness.
>> @@ -7602,7 +7614,6 @@ build_cxx_call (tree fn, int nargs, tree *argarray,
>>&& current_function_decl
>>&& DECL_DECLARED_CONSTEXPR_P (current_function_decl))
>> optimize = 1;
>> - fn = fold_if_not_in_template (fn);
>>optimize = optimize_sav;
>
>
> Since we're removing the fold, we can also remove the changes to "optimize".
Yeah, I missed that. I removed superfluous code-path and committed it
on branch.
>> @@ -443,7 +443,7 @@ build_base_path (enum tree_code code,
>>
>> t = TREE_TYPE (TYPE_VFIELD (current_class_type));
>> t = build_pointer_type (t);
>> - v_offset = convert (t, current_vtt_parm);
>> + v_offset = fold (convert (t, current_vtt_parm));
>
> fold_convert should work here.
Well, fold_const isn't necessarily the same as fold (convert ())
within C++, due convert handles special cases fold_const doesn't. At
least I ran here into issues for templates within templates AFAIR.
>> @@ -576,7 +576,6 @@ build_simple_base_path (tree expr, tree binfo)
>> expr = build3 (COMPONENT_REF,
>>cp_build_qualified_type (type, type_quals),
>>expr, field, NULL_TREE);
>> - expr = fold_if_not_in_template (expr);
>
>
> I don't think we need to remove this fold, since it is part of compil
C++ delayed folding branch review
+ expr = fold (expr);
/* This may happen, because for LHS op= RHS we preevaluate
RHS and create C_MAYBE_CONST_EXPR >, which
means we could no longer see the code of the EXPR. */
if (TREE_CODE (expr) == C_MAYBE_CONST_EXPR)
expr = C_MAYBE_CONST_EXPR_EXPR (expr);
if (TREE_CODE (expr) == SAVE_EXPR)
-expr = TREE_OPERAND (expr, 0);
+expr = fold (TREE_OPERAND (expr, 0));
How about moving the first fold after the SAVE_EXPR block, so that we
only need to call fold once?
+case NEGATE_EXPR:
+case BIT_NOT_EXPR:
+case CONVERT_EXPR:
+case VIEW_CONVERT_EXPR:
+case NOP_EXPR:
+case FIX_TRUNC_EXPR:
+ {
+ tree op1 = TREE_OPERAND (expr, 0);
+ tree fop1 = fold (op1);
+ if (fop1 && op1 != fop1)
+ fop1 = fold_build1_loc (loc, TREE_CODE (expr), TREE_TYPE (expr),
+ fop1);
Isn't this redundant with the call to fold above? If not, it seems that
the above call should be to *_fully_fold. I suppose we want an entry
point defined by both front ends that c-common code can call which does
full folding of an expression.
@@ -597,9 +597,9 @@ null_member_pointer_value_p (tree t)
return false;
else if (TYPE_PTRMEMFUNC_P (type))
return (TREE_CODE (t) == CONSTRUCTOR
- && integer_zerop (CONSTRUCTOR_ELT (t, 0)->value));
+ && integer_zerop (fold (CONSTRUCTOR_ELT (t, 0)->value)));
else if (TYPE_PTRDATAMEM_P (type))
-return integer_all_onesp (t);
+return integer_all_onesp (fold (t));
Calling fold here is wrong; it doesn't handle constexpr, and we should
have folded before we got here.
warn_logical_operator (loc, code, boolean_type_node,
- code_orig_arg1, arg1,
- code_orig_arg2, arg2);
+ code_orig_arg1, fold (arg1),
+ code_orig_arg2, fold (arg2));
I think warn_logical_operator should call back into *_fully_fold.
Likewise for most similar added calls to fold.
@@ -7356,8 +7354,13 @@ build_over_call (struct z_candidate *cand, int flags, tsu
bst_flags_t complain)
gcc_assert (j <= nargs);
nargs = j;
+ {
+tree *fargs = (!nargs ? argarray : (tree *) alloca (nargs * sizeof (tree)))
;
+for (j = 0; j < nargs; j++)
+ fargs[j] = fold_non_dependent_expr (argarray[j]);
Similarly, this and build_cxx_call should use cp_fully_fold.
@@ -7602,7 +7614,6 @@ build_cxx_call (tree fn, int nargs, tree *argarray,
&& current_function_decl
&& DECL_DECLARED_CONSTEXPR_P (current_function_decl))
optimize = 1;
- fn = fold_if_not_in_template (fn);
optimize = optimize_sav;
Since we're removing the fold, we can also remove the changes to "optimize".
@@ -443,7 +443,7 @@ build_base_path (enum tree_code code,
t = TREE_TYPE (TYPE_VFIELD (current_class_type));
t = build_pointer_type (t);
- v_offset = convert (t, current_vtt_parm);
+ v_offset = fold (convert (t, current_vtt_parm));
fold_convert should work here.
@@ -576,7 +576,6 @@ build_simple_base_path (tree expr, tree binfo)
expr = build3 (COMPONENT_REF,
cp_build_qualified_type (type, type_quals),
expr, field, NULL_TREE);
- expr = fold_if_not_in_template (expr);
I don't think we need to remove this fold, since it is part of compiler
internals rather than something the user wrote. Really, we should
represent the base conversion with something like a CONVERT_EXPR and
only call this function when we want to fold it. But that can wait for
a later patch.
@@ -1046,6 +1048,9 @@ adjust_temp_type (tree type, tree temp)
{
if (TREE_TYPE (temp) == type)
return temp;
+ STRIP_NOPS (temp);
+ if (TREE_TYPE (temp) == type)
+return temp;
...
reduced_constant_expression_p (tree t)
{
+ /* Make sure we remove useless initial NOP_EXPRs. */
+ STRIP_NOPS (t);
Where are these NOPs coming from?
@@ -1082,7 +1087,10 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx,
tree t,
&& is_dummy_object (x))
{
x = ctx->object;
- x = cp_build_addr_expr (x, tf_warning_or_error);
+ if (x)
+ x = cp_build_addr_expr (x, tf_warning_or_error);
+ else
+ x = get_nth_callarg (t, i);
This should not be necessary.
@@ -1765,7 +1780,8 @@ cxx_eval_component_reference (const constexpr_ctx *ctx,
tree t,
if (field == part)
{
if (value)
- return value;
+ return cxx_eval_constant_expression (ctx, value, lval,
+non_constant_p, overflow_p);
...
@@ -1849,7 +1865,8 @@ cxx_eval_bit_field_ref (const constexpr_ctx *ctx, tree t,
{
tree bitpos = bit_position (field);
if (bitpos == start && DECL_SIZE (field) == TREE_OPERAND (t, 1))
- return value;
+ return cxx
