Skip to content

Commit 12de194

Browse files
mpolacekPatrick Palka
and
Patrick Palka
committed
c++: compile time evaluation of prvalues [PR116416]
This PR reports a missed optimization. When we have: Str str{"Test"}; callback(str); as in the test, we're able to evaluate the Str::Str() call at compile time. But when we have: callback(Str{"Test"}); we are not. With this patch (in fact, it's Patrick's patch with a little tweak), we turn callback (TARGET_EXPR <D.2890, <<< Unknown tree: aggr_init_expr 5 __ct_comp D.2890 (struct Str *) <<< Unknown tree: void_cst >>> (const char *) "Test" >>>>) into callback (TARGET_EXPR <D.2890, {.str=(const char *) "Test", .length=4}>) I explored the idea of calling maybe_constant_value for the whole TARGET_EXPR in cp_fold. That has three problems: - we can't always elide a TARGET_EXPR, so we'd have to make sure the result is also a TARGET_EXPR; - the resulting TARGET_EXPR must have the same flags, otherwise Bad Things happen; - getting a new slot is also problematic. I've seen a test where we had "TARGET_EXPR<D.2680, ...>, D.2680", and folding the whole TARGET_EXPR would get us "TARGET_EXPR<D.2681, ...>", but since we don't see the outer D.2680, we can't replace it with D.2681, and things break. With this patch, two tree-ssa tests regressed: pr78687.C and pr90883.C. FAIL: g++.dg/tree-ssa/pr90883.C scan-tree-dump dse1 "Deleted redundant store: .*.a = {}" is easy. Previously, we would call C::C, so .gimple has: D.2590 = {}; C::C (&D.2590); D.2597 = D.2590; return D.2597; Then .einline inlines the C::C call: D.2590 = {}; D.2590.a = {}; // rust-lang#1 D.2590.b = 0; // rust-lang#2 D.2597 = D.2590; D.2590 ={v} {CLOBBER(eos)}; return D.2597; then rust-lang#2 is removed in .fre1, and rust-lang#1 is removed in .dse1. So the test passes. But with the patch, .gimple won't have that C::C call, so the IL is of course going to look different. The .optimized dump looks the same though so there's no problem. pr78687.C is XFAILed because the test passes with r15-5746 but not with r15-5747 as well. I opened <https://gcc.gnu.org/PR117971>. PR c++/116416 gcc/cp/ChangeLog: * cp-gimplify.cc (cp_fold_r) <case TARGET_EXPR>: Try to fold TARGET_EXPR_INITIAL and replace it with the folded result if it's TREE_CONSTANT. gcc/testsuite/ChangeLog: * g++.dg/analyzer/pr97116.C: Adjust dg-message. * g++.dg/tree-ssa/pr78687.C: Add XFAIL. * g++.dg/tree-ssa/pr90883.C: Adjust dg-final. * g++.dg/cpp0x/constexpr-prvalue1.C: New test. * g++.dg/cpp1y/constexpr-prvalue1.C: New test. Co-authored-by: Patrick Palka <[email protected]> Reviewed-by: Jason Merrill <[email protected]>
1 parent 0c83096 commit 12de194

File tree

6 files changed

+67
-6
lines changed

6 files changed

+67
-6
lines changed

gcc/cp/cp-gimplify.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,13 +1470,19 @@ cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data_)
14701470
if (data->flags & ff_genericize)
14711471
cp_genericize_target_expr (stmt_p);
14721472

1473-
/* Folding might replace e.g. a COND_EXPR with a TARGET_EXPR; in
1474-
that case, strip it in favor of this one. */
14751473
if (tree &init = TARGET_EXPR_INITIAL (stmt))
14761474
{
14771475
cp_walk_tree (&init, cp_fold_r, data, NULL);
14781476
cp_walk_tree (&TARGET_EXPR_CLEANUP (stmt), cp_fold_r, data, NULL);
14791477
*walk_subtrees = 0;
1478+
if (!flag_no_inline)
1479+
{
1480+
tree folded = maybe_constant_init (init, TARGET_EXPR_SLOT (stmt));
1481+
if (folded != init && TREE_CONSTANT (folded))
1482+
init = folded;
1483+
}
1484+
/* Folding might replace e.g. a COND_EXPR with a TARGET_EXPR; in
1485+
that case, strip it in favor of this one. */
14801486
if (TREE_CODE (init) == TARGET_EXPR)
14811487
{
14821488
tree sub = TARGET_EXPR_INITIAL (init);

gcc/testsuite/g++.dg/analyzer/pr97116.C

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ struct foo
1616
void test_1 (void)
1717
{
1818
foo *p = new(NULL) foo (42); // { dg-warning "non-null expected" "warning" }
19-
// { dg-message "argument 'this' \\(\[^\n\]*\\) NULL where non-null expected" "final event" { target *-*-* } .-1 }
19+
// { dg-message "argument 'this'( \\(\[^\n\]*\\))? NULL where non-null expected" "final event" { target *-*-* } .-1 }
2020
}
2121

2222
int test_2 (void)
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// PR c++/116416
2+
// { dg-do compile { target c++11 } }
3+
// { dg-options "-O" }
4+
5+
struct optional {
6+
constexpr optional(int) {}
7+
};
8+
optional foo() { return 2; }
9+
10+
11+
struct C {
12+
constexpr C(int) {}
13+
};
14+
15+
struct B {
16+
C fn(int) { return 0; }
17+
};
18+
19+
void
20+
g ()
21+
{
22+
B b;
23+
b.fn(0);
24+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// PR c++/116416
2+
// { dg-do compile { target c++14 } }
3+
// { dg-options "-O" }
4+
5+
struct Str {
6+
constexpr Str() {}
7+
constexpr Str(const char *instr) {
8+
str = instr; length = 0;
9+
for (auto index = 0; instr[index]; ++index) {
10+
++length;
11+
}
12+
}
13+
const char *str = nullptr;
14+
int length = 0;
15+
};
16+
extern void callback(Str str);
17+
void
18+
func1()
19+
{
20+
callback(Str{"Test"});
21+
}
22+
void
23+
func2()
24+
{
25+
Str str{"Test"};
26+
callback(str);
27+
}
28+
29+
// Check that we don't call Str::Str(char const*)
30+
// { dg-final { scan-assembler-not "_ZN3StrC1EPKc" } }

gcc/testsuite/g++.dg/tree-ssa/pr78687.C

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,4 +480,5 @@ int main(int argc, char* argv[])
480480
f();
481481
}
482482

483-
/* { dg-final { scan-tree-dump "Removing load:.*ptr;" "sra" } } */
483+
// XFAILed due to PR116416
484+
/* { dg-final { scan-tree-dump "Removing load:.*ptr;" "sra" { xfail { *-*-* } } } } */

gcc/testsuite/g++.dg/tree-ssa/pr90883.C

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,5 @@
1616
// We want to match enough here to capture that we deleted an empty
1717
// constructor store
1818
// mips will expand to loop to clear because CLEAR_RATIO.
19-
// { dg-final { scan-tree-dump "Deleted redundant store: .*\.a = {}" "dse1" { xfail { mips*-*-* } } } }
20-
19+
// { dg-final { scan-tree-dump-not ".*\.a = {}" "dse1" { xfail { mips*-*-* } } } }
20+
// { dg-final { scan-tree-dump-not ".*\.b = 0" "dse1" { xfail { mips*-*-* } } } }

0 commit comments

Comments
 (0)