On Mon, Aug 28, 2017 at 7:46 PM, Martin Sebor <mse...@gmail.com> wrote:
> operand_equal_p() doesn't handle SSA_NAMEs and returns false for
> operands in that form even when they have equal values (when both
> are ADDR_EXPR of the same decl).  Yet the function is extensively
> relied on in the middle end where I would expect it be beneficial
> to have it handle SSA_NAMEs.  At a minimum, it would let some
> tests succeed earlier rather than later, leading to better code.
> One example is:
>
>   char a[32];
>   memcpy (&a[2], &a[4] - 2, 7);
>
> which would be eliminated even without optimization if the function
> did handle SSA_NAMEs.


It does handle it via:
  /* If ARG0 and ARG1 are the same SAVE_EXPR, they are necessarily equal.
     We don't care about side effects in that case because the SAVE_EXPR
     takes care of that for us. In all other cases, two expressions are
     equal if they have no side effects.  If we have two identical
     expressions with side effects that should be treated the same due
     to the only side effects being identical SAVE_EXPR's, that will
     be detected in the recursive calls below.
     If we are taking an invariant address of two identical objects
     they are necessarily equal as well.  */
  if (arg0 == arg1 && ! (flags & OEP_ONLY_CONST)
      && (TREE_CODE (arg0) == SAVE_EXPR
          || (flags & OEP_MATCH_SIDE_EFFECTS)
          || (! TREE_SIDE_EFFECTS (arg0) && ! TREE_SIDE_EFFECTS (arg1))))
    return 1;


That is SSA_NAMES are only equal to another SSA_NAME if it is self.
Otherwise it a different optimization pass job to get them to be
equal.
In this case most likely FRE or DOM.  If they are not doing the job,
you need to check why and not change operand_equal_p to look back it
the SSA_NAME's definition.

Thanks,
Andrew Pinski

>
> When this handling of ADDR_EXPR is added, the internal call to
> the recursive function returns true but the outermost call then
> fails the "hash state" assertion.  With the assertion removed
> the change then causes at least one test suite failure due to
> a gimple validation check.  That suggests that the function
> deliberately doesn't handle SSA_NAMEs but I don't understand
> why.
>
> If that is, in fact, intentional, can someone explain what
> the rationale is?
>
> Incidentally, I noticed that operand_equal_for_phi_arg_p() in
> tree.c that calls operand_equal_p() takes care to avoid doing so
> when either argument is an SSA_NAME, apparently as a (presumably
> compile-time) optimization.  That seems to confirm that not
> handling SSA_NAMEs is intentional (but doesn't explain it).
> (As an aside, if operand_equal_p() is meant to return false
> for SSA_NAMES and there is a use case for it doing so quickly
> why not simply have it do what operand_equal_for_phi_arg_p does
> as the first thing?)
>
> Thanks
> Martin

Reply via email to