Andrew Stubbs <a...@codesourcery.com> writes:
> On 26/09/18 17:48, Richard Sandiford wrote:
>> Andrew Stubbs <a...@codesourcery.com> writes:
>>> +  /* Nested vec_merge.  */
>>> +  rtx nvm = gen_rtx_VEC_MERGE (mode, vm1, vm2, mask1);
>>> +  ASSERT_EQ (vm1, simplify_merge_mask (nvm, mask1, 0));
>>> +  ASSERT_EQ (vm2, simplify_merge_mask (nvm, mask1, 1));
>> 
>> Think the last two should simplify to op0 and op3, which I guess
>> means recursing on the "return XEXP (x, op);"
>
> I thought about doing that, but I noticed that, for example, 
> simplify_gen_unary does not recurse into its operand. Is that an 
> omission, or is it expected that those operands will already have been 
> simplified?

Ah, yeah, each operand should already fully be simplified.  But then the
only thing we testing here compared to:

  /* Simple vec_merge.  */
  ASSERT_EQ (op0, simplify_merge_mask (vm1, mask1, 0));
  ASSERT_EQ (op1, simplify_merge_mask (vm1, mask1, 1));

is that we *don't* recurse.  It would be worth adding a comment
to say that, since if we both thought about it, I guess whoever
comes next will too.

And the assumption that existing VEC_MERGEs are fully simplified means
we should return null:

  if (GET_CODE (x) == VEC_MERGE && rtx_equal_p (XEXP (x, 2), mask))
    {
      if (!side_effects_p (XEXP (x, 1 - op)))
        return XEXP (x, op);
--->here
    }

On keeping the complexity down:

  if (side_effects_p (x))
    return NULL_RTX;

makes this quadratic for chains of unary operations.  Is it really
needed?  The code after it simply recurses on operands and doesn't
discard anything itself, so it looks like the VEC_MERGE call to
side_effects_p would be enough.

Richard

Reply via email to