On 11/13/20 4:05 AM, Richard Biener wrote:
On Thu, Nov 12, 2020 at 10:12 PM Andrew MacLeod <amacl...@redhat.com
<mailto:amacl...@redhat.com>> wrote:
On 11/12/20 3:53 PM, Richard Biener wrote:
> On November 12, 2020 9:43:52 PM GMT+01:00, Andrew MacLeod via
Gcc-patches <gcc-patches@gcc.gnu.org
<mailto:gcc-patches@gcc.gnu.org>> wrote:
>> So I spent some time tracking down a ranger issue, and in the
end, it
>> boiled down to the range-op handler not being picked up properly.
>>
>> The handler is picked up by:
>>
>> if ((gimple_code (s) == GIMPLE_ASSIGN) || (gimple_code (s) ==
>> GIMPLE_COND))
>> return range_op_handler (gimple_expr_code (s),
gimple_expr_type
>> (s));
> IMHO this should use more specific functions. Gimple_expr_code
should go away similar to gimple_expr_type.
gimple_expr_type is quite pervasive.. and each consumer is going
to have
to roll their own version of it. Why do we want to get rid of it?
If we are trying to save a few bytes by storing the information in
different places, then we're going to need some sort of accessing
function like that
>
>> where it is indexing the table with the gimple_expr_code..
>> the stmt being processed was for a pointer assignment,
>> _5 = _33
>> and it was coming back with a gimple_expr_code of VAR_DECL
instead of
>> an SSA_NAME... which confused me greatly.
>>
>>
>> gimple_expr_code (const gimple *stmt)
>> {
>> enum gimple_code code = gimple_code (stmt);
>> if (code == GIMPLE_ASSIGN || code == GIMPLE_COND)
>> return (enum tree_code) stmt->subcode;
>>
>> A little more digging shows this:
>>
>> static inline enum tree_code
>> gimple_assign_rhs_code (const gassign *gs)
>> {
>> enum tree_code code = (enum tree_code) gs->subcode;
>> /* While we initially set subcode to the TREE_CODE of the
rhs for
>> GIMPLE_SINGLE_RHS assigns we do not update that subcode
to stay
>> in sync when we rewrite stmts into SSA form or do SSA
>> propagations. */
>> if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS)
>> code = TREE_CODE (gs->op[1]);
>>
>> return code;
>> }
>>
>> Fascinating comment.
> ... 😬
>
>> But it means that gimple_expr_code() isn't returning the
correct result
>>
>> for GIMPLE_SINGLE_RHS....
> It depends. A SSA name isn't an expression code either. As said,
the generic gimple_expr_code should be used with extreme care.
what is an expression code? It seems like its just a tree_code
representing what is on the RHS? Im not sure I understand why one
needs to be careful with it. It only applies to COND, ASSIGN and
CALL.
and its current right for everything except GIMPLE_SINGLE_RHS?
If we dont fix gimple_expr_code, then Im basically going to be
reimplementing it myself... which seems kind of pointless.
Well sure we can fix it. Your patch looks OK but can be optimized like
if (gassign *ass = dyn_cast<gassign *> (stmt))
return gimple_assign_rhs_code (stmt);
note it looks odd that we use this for gimple_assign but
directly access subcode for GIMPLE_COND instead
of returning gimple_cond_code () (again, operate on
gcond to avoid an extra check).
Thanks,
Richard.
Andrew
And with a little bit of const-ness... I adjusted gimple_range_handler
to use gassing and gcond as well.
Bootstrapped on x86_64-pc-linux-gnu, no regressions. pushed.
Andrew
commit fcbb6018abaf04d30e2cf6fff2eb35115412cdd5
Author: Andrew MacLeod <amacl...@redhat.com>
Date: Fri Nov 13 13:56:01 2020 -0500
Re: Fix gimple_expr_code?
have gimple_expr_code return the correct code for GIMPLE_ASSIGN.
use gassign and gcond in gimple_range_handler.
* gimple-range.h (gimple_range_handler): Cast to gimple stmt
kinds before asking for code and type.
* gimple.h (gimple_expr_code): Call gassign and gcond routines
to get their expr_code.
diff --git a/gcc/gimple-range.h b/gcc/gimple-range.h
index dde41e9e743..92bb5305c18 100644
--- a/gcc/gimple-range.h
+++ b/gcc/gimple-range.h
@@ -97,12 +97,12 @@ extern bool gimple_range_calc_op2 (irange &r, const gimple *s,
static inline range_operator *
gimple_range_handler (const gimple *s)
{
- if (gimple_code (s) == GIMPLE_ASSIGN)
- return range_op_handler (gimple_assign_rhs_code (s),
- TREE_TYPE (gimple_assign_lhs (s)));
- if (gimple_code (s) == GIMPLE_COND)
- return range_op_handler (gimple_cond_code (s),
- TREE_TYPE (gimple_cond_lhs (s)));
+ if (const gassign *ass = dyn_cast<const gassign *> (s))
+ return range_op_handler (gimple_assign_rhs_code (ass),
+ TREE_TYPE (gimple_assign_lhs (ass)));
+ if (const gcond *cond = dyn_cast<const gcond *> (s))
+ return range_op_handler (gimple_cond_code (cond),
+ TREE_TYPE (gimple_cond_lhs (cond)));
return NULL;
}
diff --git a/gcc/gimple.h b/gcc/gimple.h
index d359f7827b1..b935ad4f761 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -2229,26 +2229,6 @@ gimple_set_modified (gimple *s, bool modifiedp)
}
-/* Return the tree code for the expression computed by STMT. This is
- only valid for GIMPLE_COND, GIMPLE_CALL and GIMPLE_ASSIGN. For
- GIMPLE_CALL, return CALL_EXPR as the expression code for
- consistency. This is useful when the caller needs to deal with the
- three kinds of computation that GIMPLE supports. */
-
-static inline enum tree_code
-gimple_expr_code (const gimple *stmt)
-{
- enum gimple_code code = gimple_code (stmt);
- if (code == GIMPLE_ASSIGN || code == GIMPLE_COND)
- return (enum tree_code) stmt->subcode;
- else
- {
- gcc_gimple_checking_assert (code == GIMPLE_CALL);
- return CALL_EXPR;
- }
-}
-
-
/* Return true if statement STMT contains volatile operands. */
static inline bool
@@ -3793,6 +3773,28 @@ gimple_cond_set_condition (gcond *stmt, enum tree_code code, tree lhs,
gimple_cond_set_rhs (stmt, rhs);
}
+
+/* Return the tree code for the expression computed by STMT. This is
+ only valid for GIMPLE_COND, GIMPLE_CALL and GIMPLE_ASSIGN. For
+ GIMPLE_CALL, return CALL_EXPR as the expression code for
+ consistency. This is useful when the caller needs to deal with the
+ three kinds of computation that GIMPLE supports. */
+
+static inline enum tree_code
+gimple_expr_code (const gimple *stmt)
+{
+ if (const gassign *ass = dyn_cast<const gassign *> (stmt))
+ return gimple_assign_rhs_code (ass);
+ if (const gcond *cond = dyn_cast<const gcond *> (stmt))
+ return gimple_cond_code (cond);
+ else
+ {
+ gcc_gimple_checking_assert (gimple_code (stmt) == GIMPLE_CALL);
+ return CALL_EXPR;
+ }
+}
+
+
/* Return the LABEL_DECL node used by GIMPLE_LABEL statement GS. */
static inline tree
