[PATCH] Re: Fix gimple_expr_code?

Andrew MacLeod amacleod@redhat.com
Fri Nov 13 19:00:29 GMT 2020


On 11/13/20 4:05 AM, Richard Biener wrote:
> On Thu, Nov 12, 2020 at 10:12 PM Andrew MacLeod <amacleod@redhat.com 
> <mailto:amacleod@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: expr.diff
Type: text/x-patch
Size: 3348 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20201113/7c60bb4c/attachment.bin>


More information about the Gcc-patches mailing list