[PATCH] Fix tree-opt/110252: wrong code due to phiopt using flow sensitive info during match

Match will query ranger via tree_nonzero_bits/get_nonzero_bits for 2 and 3rd
operand of the COND_EXPR and phiopt tries to do create the COND_EXPR even if we moving
one statement. That one statement could have some flow sensitive information on it
based on the condition that is for the COND_EXPR but that might create wrong code
if the statement was moved out.

This is similar to the previous version of the patch except now we use
flow_sensitive_info_storage instead of manually doing the save/restore
and also handle all defs on a gimple statement rather than just for lhs
of the gimple statement. Oh and a few more testcases were added that
was failing before.

OK? Bootsrapped and tested on x86_64-linux-gnu with no regressions.

	PR tree-optimization/110252

gcc/ChangeLog:

	* tree-ssa-phiopt.cc (class auto_flow_sensitive): New class.
	(auto_flow_sensitive::auto_flow_sensitive): New constructor.
	(auto_flow_sensitive::~auto_flow_sensitive): New deconstructor.
	(match_simplify_replacement): Temporarily
	remove the flow sensitive info on the two statements that might
	be moved.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/phi-opt-25b.c: Updated as
	__builtin_parity loses the nonzerobits info.
	* gcc.c-torture/execute/pr110252-1.c: New test.
	* gcc.c-torture/execute/pr110252-2.c: New test.
	* gcc.c-torture/execute/pr110252-3.c: New test.
	* gcc.c-torture/execute/pr110252-4.c: New test.
This commit is contained in:
Andrew Pinski 2023-07-14 15:55:34 -07:00
parent a86d5eca6a
commit 8c79b49cd4
6 changed files with 96 additions and 7 deletions

View file

@ -0,0 +1,15 @@
/* This is reduced from sel-sched.cc which was noticed was being miscompiled too. */
int g(int min_need_stall) __attribute__((__noipa__));
int g(int min_need_stall)
{
return min_need_stall < 0 ? 1 : ((min_need_stall) < (1) ? (min_need_stall) : (1));
}
int main(void)
{
for(int i = -100; i <= 100; i++)
{
int t = g(i);
if (t != (i!=0))
__builtin_abort();
}
}

View file

@ -0,0 +1,10 @@
signed char f() __attribute__((__noipa__));
signed char f() { return 0; }
int main()
{
int g = f() - 1;
int e = g < 0 ? 1 : ((g >> (8-2))!=0);
asm("":"+r"(e));
if (e != 1)
__builtin_abort();
}

View file

@ -0,0 +1,13 @@
unsigned int a = 1387579096U;
void sinkandcheck(unsigned b) __attribute__((noipa));
void sinkandcheck(unsigned b)
{
if (a != b)
__builtin_abort();
}
int main() {
a = 1 < (~a) ? 1 : (~a);
sinkandcheck(1);
return 0;
}

View file

@ -0,0 +1,8 @@
int a, b = 2, c = 2;
int main() {
b = ~(1 % (a ^ (b - (1 && c) || c & b)));
if (b < -1)
__builtin_abort();
return 0;
}

View file

@ -65,8 +65,6 @@ int test_popcountll(unsigned long long x, unsigned long long y)
return x ? __builtin_popcountll(y) : 0;
}
/* 3 types of functions (not including parity), each with 3 types and there are 2 goto each */
/* { dg-final { scan-tree-dump-times "goto " 18 "optimized" } } */
/* 4 types of functions, each with 3 types and there are 2 goto each */
/* { dg-final { scan-tree-dump-times "goto " 24 "optimized" } } */
/* { dg-final { scan-tree-dump-times "x_..D. != 0" 12 "optimized" } } */
/* parity case will be optimized to x!=0 & parity(y) . */
/* { dg-final { scan-tree-dump-times " & " 3 "optimized" } } */

View file

@ -708,6 +708,45 @@ move_stmt (gimple *stmt, gimple_stmt_iterator *gsi, auto_bitmap &inserted_exprs)
reset_flow_sensitive_info (name);
}
/* RAII style class to temporarily remove flow sensitive
from ssa names defined by a gimple statement. */
class auto_flow_sensitive
{
public:
auto_flow_sensitive (gimple *s);
~auto_flow_sensitive ();
private:
auto_vec<std::pair<tree, flow_sensitive_info_storage>, 2> stack;
};
/* Constructor for auto_flow_sensitive. Saves
off the ssa names' flow sensitive information
that was defined by gimple statement S and
resets it to be non-flow based ones. */
auto_flow_sensitive::auto_flow_sensitive (gimple *s)
{
if (!s)
return;
ssa_op_iter it;
tree def;
FOR_EACH_SSA_TREE_OPERAND (def, s, it, SSA_OP_DEF)
{
flow_sensitive_info_storage storage;
storage.save_and_clear (def);
stack.safe_push (std::make_pair (def, storage));
}
}
/* Deconstructor, restores the flow sensitive information
for the SSA names that had been saved off. */
auto_flow_sensitive::~auto_flow_sensitive ()
{
for (auto p : stack)
p.second.restore (p.first);
}
/* The function match_simplify_replacement does the main work of doing the
replacement using match and simplify. Return true if the replacement is done.
Otherwise return false.
@ -793,9 +832,15 @@ match_simplify_replacement (basic_block cond_bb, basic_block middle_bb,
return false;
tree type = TREE_TYPE (gimple_phi_result (phi));
result = gimple_simplify_phiopt (early_p, type, stmt,
arg_true, arg_false,
&seq);
{
auto_flow_sensitive s1(stmt_to_move);
auto_flow_sensitive s_alt(stmt_to_move_alt);
result = gimple_simplify_phiopt (early_p, type, stmt,
arg_true, arg_false,
&seq);
}
if (!result)
return false;