This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Enhance phiopt to handle BIT_AND_EXPR
- From: Jeff Law <law at redhat dot com>
- To: Andrew Pinski <pinskia at gmail dot com>, Zhenqiang Chen <zhenqiang dot chen at arm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 08 Oct 2013 15:00:15 -0600
- Subject: Re: [PATCH] Enhance phiopt to handle BIT_AND_EXPR
- Authentication-results: sourceware.org; auth=none
- References: <000001cebdbf$a3155310$e93ff930$ at arm dot com> <CA+=Sn1mP8j0Oanhmfu2-ATZ8yAc_z4_5B0T1f7v2zahE16mPxA at mail dot gmail dot com>
On 09/30/13 09:57, Andrew Pinski wrote:
Agreed. Repeating blobs of code like that should be pulled out into its
On Mon, Sep 30, 2013 at 2:29 AM, Zhenqiang Chen <firstname.lastname@example.org> wrote:
The patch enhances phiopt to handle cases like:
if (a == 0 && (...))
Boot strap and no make check regression on X86-64 and ARM.
Is it OK for trunk?
From someone who wrote lot of this code (value_replacement in fact),
this looks good, though I would pull:
+ if (TREE_CODE (gimple_assign_rhs1 (def)) == SSA_NAME)
+ gimple def1 = SSA_NAME_DEF_STMT (gimple_assign_rhs1 (def));
+ if (is_gimple_assign (def1) && gimple_assign_rhs_code (def1) == EQ_EXPR)
+ tree op0 = gimple_assign_rhs1 (def1);
+ tree op1 = gimple_assign_rhs2 (def1);
+ if ((operand_equal_for_phi_arg_p (arg0, op0)
+ && operand_equal_for_phi_arg_p (arg1, op1))
+ || (operand_equal_for_phi_arg_p (arg0, op1)
+ && operand_equal_for_phi_arg_p (arg1, op0)))
+ *code = gimple_assign_rhs_code (def1);
+ return 1;
Out into its own function since it is repeated again for
It's been 10 years since we wrote that code Andrew and in looking at it
again, I wonder if/why DOM doesn't handle the stuff in
value_replacement. It really just looks like it's propagation of an
edge equivalence. do you recall the motivation behind value_replacement
and why we didn't just let DOM handle it?
Also what about cascading BIT_AND_EXPR
Well, given the structure of how that code is going to look, it'd just
be repeated walking through the input chains. Do-able, yes. But I
don't think handling that should be a requirement for integration.
if((a == 0) & (...) & (...))
I notice you don't handle that either.
I'll go ahead and pull the common bits into a single function and commit
on Zhenqiang's behalf.