Re: [PATCH] simplify-rtx.c: start adding selftests (v2)

[David Malcolm <dmalcolm at redhat dot com>]

On Wed, 2016-07-13 at 13:38 -0600, Jeff Law wrote:
> On 07/06/2016 01:30 PM, David Malcolm wrote:
> > > 
> > > This might be a bit confusing when more tests are added, since
> > > pointer equality is only useful in certain specific cases (e.g.
> > > when you know you're dealing with CONST_INTs or pseudo
> > > registers).
> > > How about making ASSERT_RTX_EQ check for rtx_equal_p equality and
> > > have something like ASSERT_RTX_PTR_EQ for cases where pointer
> > > equality really is needed?
> > 
> > > Also, how about using LAST_VIRTUAL_REGISTER + 1 as the base for
> > > register numbers?  DImode might not be valid for register 0 on
> > > all
> > > targets.
> > 
> > Thanks.  Here's an updated version which adds both ASSERT_RTX_EQ
> > and
> > ASSERT_RTX_PTR_EQ.  The simplify-rtx.c tests can use the stricter
> > pointer equality test, so I updated them to use ASSERT_RTX_PTR_EQ
> > condition.
> Richard S. is definitely right here WRT using pointer equality vs
> deeper 
> inspection.   The RTL structure sharing rules within GCC are
> something 
> you have to be cognizant of here.  Thankfully the RTL structure
> sharing 
> is reasonably well documented.
> > 
> > I added a selftest::make_test_reg to allocate pseudo regs, starting
> > at LAST_VIRTUAL_REGISTER + 1.
> Also the right thing to do.  There's hard registers, then virtuals,
> then 
> pseudos.
> 
> There's obviously much more we could do with the tests, but this is a
> reasonable start.
> 
> I note you iterate over all the modes -- which would include things
> like 
> FP modes, BImode, and CC special modes and such.  I don't think we
> can 
> necessarily be sure that the transformations you're testing are valid
> across all modes.
> 
> For example, does it even make sense to test (A & B) | A -> A for FP
> modes?
> 
> It almost seems like the iteration space has to be dependent on what 
> you're testing.  Ie, some tests you want to iterate over the standard
> integer modes.  Other tests you might reasonably include FP modes. 
>  CC 
> modes I think should be forbidden for these tests.  THere may be
> others 
> to ponder as well.

Thanks.  My thinking here is to have the iteration over all modes, and
then filter it within the tests, with something like this at the top of
a test:

  if (!INTEGRAL_MODE_P (mode))
     return;

That said, and this is probably my relative unfamiliarity with RTL
speaking, but I'm not sure exactly how the modes should be filtered.

For example, my naive "run in all modes" approach ran into the issue
that although (A + 0) -> A works, (0 + A) -> A currently doesn't work
for complex modes (where "0" is CONST0_RTX(mode)).  Is that a bug?  If
so, since the run-in-all-modes approach uncovered it, was the testing
useful?

If that's the case, would it make sense to have some kind of
NORMAL_MODE_P (mode) filter, say, or conversely SPECIAL_MODE_P (mode),
and do an early-reject in the loop over all modes?  (to reject CCmode,
VOIDmode, BImode, I think; any others?).

Dave