Re: [PATCH 8/9] Introduce class function_reader (v4)

[David Malcolm <dmalcolm@redhat.com>]

On Wed, 2016-11-23 at 21:15 +0100, Bernd Schmidt wrote:
> On 11/11/2016 10:15 PM, David Malcolm wrote:
> 
> > +static void
> > +aarch64_test_loading_full_dump ()
> > +{
> > +  rtl_dump_test t (SELFTEST_LOCATION, locate_file ("aarch64/times
> > -two.rtl"));
> > +
> > +  ASSERT_STREQ ("times_two", IDENTIFIER_POINTER (DECL_NAME (cfun
> > ->decl)));
> > +
> > +  rtx_insn *insn_1 = get_insn_by_uid (1);
> > +  ASSERT_EQ (NOTE, GET_CODE (insn_1));
> > +
> > +  rtx_insn *insn_15 = get_insn_by_uid (15);
> > +  ASSERT_EQ (INSN, GET_CODE (insn_15));
> > +  ASSERT_EQ (USE, GET_CODE (PATTERN (insn_15)));
> > +
> > +  /* Verify crtl->return_rtx.  */
> > +  ASSERT_EQ (REG, GET_CODE (crtl->return_rtx));
> > +  ASSERT_EQ (0, REGNO (crtl->return_rtx));
> > +  ASSERT_EQ (SImode, GET_MODE (crtl->return_rtx));
> > +}
> 
> The problem I'm having with this patch, and why I've not really 
> commented on the latter parts of the series, is that I was hoping the
> tests would be more self-contained.

The tests in this patch (8/9) are more about verifying that the loader
is sane: they load a dump, and then assert various properties of it, to
ensure that we're loading things correctly.  I moved the example dumps
out of the .c files (using locate_file) to avoid having to embed them
in the .c file and escape things (which people objected to in an
earlier version of the kit).

> For transformations I had mentioned 
> the idea of having both before and after dumps.
> 
> Having a few test files with C code to verify them is probably not a
> big 
> deal, but I wonder whether this is going to be the norm rather than
> the 
> exception, and whether there are better ways of doing it.

These will be the exception; they're really just about verifying that
the loader is working correctly.

In a previous version of the kit there were indeed selftests that
loaded a dump and attempted to run parts of a pass on it.  I've deleted
all of those style of selftest.

> Maybe there 
> needs to be a way of annotating the input RTL to tell the compiler
> what 
> kinds of tests to run on it.

The "real" tests are in patch 9 of 9, and use DejaGnu directives
(scanning dumps, and "dg-do run" to actually run the generated code).