[Bug tree-optimization/106126] [12 Regression] tree check fail in useless_type_conversion_p, at gimple-expr.cc:87 since r13-1184-g57424087e82db140
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106126 --- Comment #16 from CVS Commits --- The master branch has been updated by Martin Liska : https://gcc.gnu.org/g:618bac5b486832edd3f8eb3ada74740e389dfcb8 commit r13-1375-g618bac5b486832edd3f8eb3ada74740e389dfcb8 Author: Martin Liska Date: Thu Jun 30 15:00:17 2022 +0200 if-to-switch: properly allow side effects only for first condition Properly allow side effects only for a first BB in a condition chain. PR tree-optimization/106126 gcc/ChangeLog: * gimple-if-to-switch.cc (struct condition_info): Save has_side_effect. (find_conditions): Parse all BBs. (pass_if_to_switch::execute): Allow only side effects for first BB. gcc/testsuite/ChangeLog: * gcc.dg/tree-ssa/pr106126.c: New test.
[Bug tree-optimization/106126] [12 Regression] tree check fail in useless_type_conversion_p, at gimple-expr.cc:87 since r13-1184-g57424087e82db140
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106126 David Binderman changed: What|Removed |Added CC||dcb314 at hotmail dot com --- Comment #15 from David Binderman --- Created attachment 53230 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53230=edit gzipped C++ source code Please find attached an additional C++ test case. -O3 required this time: $ /home/dcb/gcc/results/bin/g++ -c -O3 q1.ii during GIMPLE pass: iftoswitch ../src/common/uri.cpp: In member function ‘const char* wxURI::ParseUserInfo(const char*)’: ../src/common/uri.cpp:1086:1: internal compiler error: Segmentation fault 0x10ba099 crash_signal(int) ../../trunk.git/gcc/toplev.cc:322 0x134244a gimple_nop_p(gimple const*) ../../trunk.git/gcc/gimple.h:6747 0x134244a verify_use(basic_block_def*, basic_block_def*, ssa_use_operand_t*, gimple*, bool, bitmap_head*) ../../trunk.git/gcc/tree-ssa.cc:880
[Bug tree-optimization/106126] [12 Regression] tree check fail in useless_type_conversion_p, at gimple-expr.cc:87 since r13-1184-g57424087e82db140
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106126 Martin Liška changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #14 from Martin Liška --- (In reply to luoxhu from comment #13) > Otherwise we need record first_bb when conditions_in_bbs->is_empty, then > check that in is_beneficial, ordered_remove the info entry if that bb is not > the first "if condition" with side_effect statement in it, the fix would be No, we need to record if a BB has a side effect and allow only side effects for a first BB in the main loop in pass_if_to_switch::execute. I'm testing a patch candidate. > as below, but I am not sure whether it is worth way doing this to > handle both PR105740 and PR106126? Well, I replied in PR105740 where the problem is one needs to run the pass later.
[Bug tree-optimization/106126] [12 Regression] tree check fail in useless_type_conversion_p, at gimple-expr.cc:87 since r13-1184-g57424087e82db140
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106126
--- Comment #13 from luoxhu at gcc dot gnu.org ---
Otherwise we need record first_bb when conditions_in_bbs->is_empty, then check
that in is_beneficial, ordered_remove the info entry if that bb is not the
first "if condition" with side_effect statement in it, the fix would be as
below, but I am not sure whether it is worth way doing this to handle
both PR105740 and PR106126?
git diff
diff --git a/gcc/gimple-if-to-switch.cc b/gcc/gimple-if-to-switch.cc
index f7b0b02628b..44bb0228856 100644
--- a/gcc/gimple-if-to-switch.cc
+++ b/gcc/gimple-if-to-switch.cc
@@ -63,7 +63,7 @@ struct condition_info
condition_info (gcond *cond): m_cond (cond), m_bb (gimple_bb (cond)),
m_forwarder_bb (NULL), m_ranges (), m_true_edge (NULL), m_false_edge
(NULL),
-m_true_edge_phi_mapping (), m_false_edge_phi_mapping ()
+m_true_edge_phi_mapping (), m_false_edge_phi_mapping (), first_bb(false)
{
m_ranges.create (0);
}
@@ -80,6 +80,7 @@ struct condition_info
edge m_false_edge;
mapping_vec m_true_edge_phi_mapping;
mapping_vec m_false_edge_phi_mapping;
+ bool first_bb;
};
/* Recond PHI mapping for an original edge E and save these into vector VEC.
*/
@@ -194,6 +195,16 @@ if_chain::is_beneficial ()
auto_vec clusters;
clusters.create (m_entries.length ());
+ for (unsigned i = 0; i < m_entries.length (); i++)
+{
+ condition_info *info = m_entries[i];
+ if (info->first_bb && i != 0 && !no_side_effect_bb (info->m_bb))
+ {
+ m_entries.ordered_remove (i);
+ break;
+ }
+}
+
for (unsigned i = 0; i < m_entries.length (); i++)
{
condition_info *info = m_entries[i];
@@ -397,6 +408,8 @@ find_conditions (basic_block bb,
tree_code code = gimple_cond_code (cond);
condition_info *info = new condition_info (cond);
+ if (conditions_in_bbs->is_empty ())
+info->first_bb = true;
[Bug tree-optimization/106126] [12 Regression] tree check fail in useless_type_conversion_p, at gimple-expr.cc:87 since r13-1184-g57424087e82db140
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106126 --- Comment #12 from luoxhu at gcc dot gnu.org --- conditions_in_bbs->is_empty doesn't mean that range is at the start of switch condition:(, so couldn't assume to ignore the no_side_effect_bb check?
[Bug tree-optimization/106126] [12 Regression] tree check fail in useless_type_conversion_p, at gimple-expr.cc:87 since r13-1184-g57424087e82db140
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106126
--- Comment #11 from luoxhu at gcc dot gnu.org ---
Sorry for breaking, my bugzilla account is luo...@gcc.gnu.org.
The patch seems reasonable to fold 65-90 ('A'-'Z') to switch statement,
4,6c4,6
< ;; Canonical GIMPLE case clusters: 33 60 62 126
< ;; BT can be built: BT(values:3 comparisons:6 range:30 density: 20.00%):33-62
126
< pr106126.c:3:28: optimized: Condition chain with 4 BBs transformed into a
switch statement.
---
> ;; Canonical GIMPLE case clusters: 33 60 62 65-90 126
> ;; BT can be built: BT(values:3 comparisons:6 range:30 density: 20.00%):33-62
> 65-90 126
> pr106126.c:3:28: optimized: Condition chain with 5 BBs transformed into a
> switch statement.
...
96,97c108,109
<:
< switch (_13) [INV], case 33: [INV], case 60:
[INV], case 62: [INV], case 126: [INV]>
---
>:
> switch (_13) [INV], case 33: [INV], case 60:
> [INV], case 62: [INV], case 65 ... 90: [INV], case 126:
> [INV]>
complete pr106126.bad.c.046t.iftoswitch:
;; Function pool_conda_matchspec (pool_conda_matchspec, funcdef_no=0,
decl_uid=1979, cgraph_uid=1, symbol_order=1)
;; Canonical GIMPLE case clusters: 33 60 62 65-90 126
;; BT can be built: BT(values:3 comparisons:6 range:30 density: 20.00%):33-62
65-90 126
pr106126.c:3:28: optimized: Condition chain with 5 BBs transformed into a
switch statement.
Removing basic block 9
;; basic block 9, loop depth 2
;; pred:
if (_13 != 62)
goto ; [INV]
else
goto ; [INV]
;; succ: 10
;; 12
Removing basic block 10
;; basic block 10, loop depth 2
;; pred:
if (_13 != 33)
goto ; [INV]
else
goto ; [INV]
;; succ: 11
;; 12
Removing basic block 11
;; basic block 11, loop depth 2
;; pred:
if (_13 != 126)
goto ; [INV]
else
goto ; [INV]
;; succ: 3
;; 12
Removing basic block 3
;; basic block 3, loop depth 2
;; pred:
_3 = (unsigned char) _13;
_4 = _3 + 191;
if (_4 <= 25)
goto ; [INV]
else
goto ; [INV]
;; succ: 14
;; 13
Expanded into a new gimple STMT: switch (_13) [INV], case 33:
[INV], case 60: [INV], case 62: [INV], case 65 ... 90:
[INV], case 126: [INV]>
Removing basic block 13
;; basic block 13, loop depth 2
;; pred:
:
goto ; [100.00%]
;; succ: 6
Removing basic block 14
;; basic block 14, loop depth 1
;; pred:
:
;; succ: 4
fix_loop_structure: fixing up loops for function
void pool_conda_matchspec ()
{
unsigned char _8;
char _10;
char * var_1.3_11;
char _13;
unsigned char _14;
char * var_1.3_15;
:
goto ; [INV]
:
# _14 = PHI <_3(7)>
# var_1.3_15 = PHI
:
_8 = _14 + 65;
_10 = (char) _8;
*var_1.3_15 = _10;
:
:
:
var_1.3_11 = var_1;
if (var_1.3_11 != 0B)
goto ; [INV]
else
goto ; [INV]
:
_13 = *var_1.3_11;
if (_13 != 0)
goto ; [INV]
else
goto ; [INV]
:
switch (_13) [INV], case 33: [INV], case 60:
[INV], case 62: [INV], case 65 ... 90: [INV], case 126:
[INV]>
:
:
return;
_8 = _14 + 65;
_10 = (char) _8;
*var_1.3_15 = _10;
:
:
:
var_1.3_11 = var_1;
if (var_1.3_11 != 0B)
goto ; [INV]
else
goto ; [INV]
:
_13 = *var_1.3_11;
if (_13 != 0)
goto ; [INV]
else
goto ; [INV]
:
switch (_13) [INV], case 33: [INV], case 60:
[INV], case 62: [INV], case 65 ... 90: [INV], case 126:
[INV]>
:
:
return;
}
The problem is _3 is removed in basic block 3, but _14 is still using it.
[Bug tree-optimization/106126] [12 Regression] tree check fail in useless_type_conversion_p, at gimple-expr.cc:87 since r13-1184-g57424087e82db140
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106126 --- Comment #10 from Martin Liška --- (In reply to David Binderman from comment #8) > I seem strangely unable to add email address xionghu...@tencent.com to this > email, > for their opinion on this bug report. Yeah, he hasn't registered a bugzilla account. I'm going to write him an email.
[Bug tree-optimization/106126] [12 Regression] tree check fail in useless_type_conversion_p, at gimple-expr.cc:87 since r13-1184-g57424087e82db140
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106126 Martin Liška changed: What|Removed |Added Summary|tree check fail in |[12 Regression] tree check |useless_type_conversion_p, |fail in |at gimple-expr.cc:87|useless_type_conversion_p, ||at gimple-expr.cc:87 since ||r13-1184-g57424087e82db140 Assignee|unassigned at gcc dot gnu.org |marxin at gcc dot gnu.org Keywords|needs-bisection | --- Comment #9 from Martin Liška --- Started with r13-1184-g57424087e82db140. I'm going to take a look.
