On Fri, Oct 07, 2016 at 03:18:07PM +0000, Bernd Edlinger wrote:
> Hi!
>
> This extends -Wint-in-bool-context to uses of enum values in boolean
> context, and fixes one place where accidentally an enum value was
> passed to a bool parameter.
>
> I excluded enum values 0 and 1 because that is used in
> gimple-ssa-strength-reduction.c, where we have enums
> which are passed in bool function arguments:
>
> enum stride_status
> {
> UNKNOWN_STRIDE = 0,
> KNOWN_STRIDE = 1
> };
>
> enum phi_adjust_status
> {
> NOT_PHI_ADJUST = 0,
> PHI_ADJUST = 1
> };
>
> enum count_phis_status
> {
> DONT_COUNT_PHIS = 0,
> COUNT_PHIS = 1
> };
>
> I would'nt use an enum in that way, but I think it is
> at least not completely wrong to do it like that...
Using enums here seems reasonable see the discussion here
http://gcc.gnu.org/ml/gcc/2016-10/msg00006.html however I don't
understand why that code makes the argument type bool instead of the
enum type. Here's an untested patch to change those functions to take
the enum type. It might make sense to change naming some, but I think
this is somewhat of an improvement as it is.
Trev
diff --git a/gcc/gimple-ssa-strength-reduction.c
b/gcc/gimple-ssa-strength-reduction.c
index 7b14b91..f61805c 100644
--- a/gcc/gimple-ssa-strength-reduction.c
+++ b/gcc/gimple-ssa-strength-reduction.c
@@ -2135,7 +2135,7 @@ incr_vec_index (const widest_int &increment)
static tree
create_add_on_incoming_edge (slsr_cand_t c, tree basis_name,
widest_int increment, edge e, location_t loc,
- bool known_stride)
+ stride_status known_stride)
{
basic_block insert_bb;
gimple_stmt_iterator gsi;
@@ -2151,7 +2151,7 @@ create_add_on_incoming_edge (slsr_cand_t c, tree
basis_name,
basis_type = TREE_TYPE (basis_name);
lhs = make_temp_ssa_name (basis_type, NULL, "slsr");
- if (known_stride)
+ if (known_stride == KNOWN_STRIDE)
{
tree bump_tree;
enum tree_code code = PLUS_EXPR;
@@ -2218,7 +2218,7 @@ create_add_on_incoming_edge (slsr_cand_t c, tree
basis_name,
static tree
create_phi_basis (slsr_cand_t c, gimple *from_phi, tree basis_name,
- location_t loc, bool known_stride)
+ location_t loc, stride_status known_stride)
{
int i;
tree name, phi_arg;
@@ -2451,7 +2451,8 @@ count_candidates (slsr_cand_t c)
candidates with the same increment, also record T_0 for subsequent use. */
static void
-record_increment (slsr_cand_t c, widest_int increment, bool is_phi_adjust)
+record_increment (slsr_cand_t c, widest_int increment,
+ phi_adjust_status is_phi_adjust)
{
bool found = false;
unsigned i;
@@ -2491,7 +2492,8 @@ record_increment (slsr_cand_t c, widest_int increment,
bool is_phi_adjust)
the count to zero. We're only processing it so it can possibly
provide an initializer for other candidates. */
incr_vec[incr_vec_len].incr = increment;
- incr_vec[incr_vec_len].count = c->basis || is_phi_adjust ? 1 : 0;
+ incr_vec[incr_vec_len].count
+ = c->basis || (is_phi_adjust == PHI_ADJUST) ? 1 : 0;
incr_vec[incr_vec_len].cost = COST_INFINITE;
/* Optimistically record the first occurrence of this increment
@@ -2500,7 +2502,7 @@ record_increment (slsr_cand_t c, widest_int increment,
bool is_phi_adjust)
Exception: increments of -1, 0, 1 never need initializers;
and phi adjustments don't ever provide initializers. */
if (c->kind == CAND_ADD
- && !is_phi_adjust
+ && is_phi_adjust == NOT_PHI_ADJUST
&& c->index == increment
&& (increment > 1 || increment < -1)
&& (gimple_assign_rhs_code (c->cand_stmt) == PLUS_EXPR
@@ -2702,7 +2704,7 @@ optimize_cands_for_speed_p (slsr_cand_t c)
static int
lowest_cost_path (int cost_in, int repl_savings, slsr_cand_t c,
- const widest_int &incr, bool count_phis)
+ const widest_int &incr, count_phis_status count_phis)
{
int local_cost, sib_cost, savings = 0;
widest_int cand_incr = cand_abs_increment (c);
@@ -2714,7 +2716,7 @@ lowest_cost_path (int cost_in, int repl_savings,
slsr_cand_t c,
else
local_cost = cost_in - c->dead_savings;
- if (count_phis
+ if (count_phis == COUNT_PHIS
&& phi_dependent_cand_p (c)
&& !cand_already_replaced (c))
{
@@ -2749,7 +2751,7 @@ lowest_cost_path (int cost_in, int repl_savings,
slsr_cand_t c,
static int
total_savings (int repl_savings, slsr_cand_t c, const widest_int &incr,
- bool count_phis)
+ count_phis_status count_phis)
{
int savings = 0;
widest_int cand_incr = cand_abs_increment (c);
@@ -2757,7 +2759,7 @@ total_savings (int repl_savings, slsr_cand_t c, const
widest_int &incr,
if (incr == cand_incr && !cand_already_replaced (c))
savings += repl_savings + c->dead_savings;
- if (count_phis
+ if (count_phis == COUNT_PHIS
&& phi_dependent_cand_p (c)
&& !cand_already_replaced (c))
{
