On Mon, 2016-09-12 at 16:10 +0200, Bernd Schmidt wrote:
> In general, it's functionality that I would very much like to have
> (although maybe it's less useful these days now that the rtl side is
> fairly static). I think there's not much sense looking too deeply at
> the
> individual patches yet; we need to first figure out what we would
> really
> like this to look like in the end.
>
> > These tests are very target-specific and were developed mostly for
> > target==x86_64, and a little for target==aarch64.
> > I put clauses like this in the various test functions, which is a
> > kludge:
> >
> > /* Only run these tests for i386. */
> > #ifndef I386_OPTS_H
> > return;
> > #endif
> >
> > Is there a better way to express this condition? (I guess I could
> > add a selftest::target_is_x86_p () predicate).
>
> My preferred solution would still be a separate selftest backend,
> which
> could be built as a cross-compiler once in a separate top-level
> directory. That way we have control over it, and maintainers of
> actual
> targets don't need to fear breaking selftests when they make changes
> to
> their ports. The downside would primarily be the time to build it.
I'm not sure I follow - this sounds like a dedicated target for
selftesting.
Would it be a separate configuration i.e. something like:
../src/configure --target=rtl-selftest
or somesuch?
> > + const char *input_dump
> > + = ("(insn 8 0 9 2 (set (reg:DI 78)\n"
> > + " (lshiftrt:DI (reg:DI 76)\n"
> > + " (const_int 32 [0x20])))\n"
> > + " ../../src/gcc/testsuite/gcc.dg/asr_div1.c:14\n"
> > + " 641 {*aarch64_lshr_sisd_or_int_di3}\n"
> > + " (expr_list:REG_DEAD (reg:DI 76)\n"
> > + " (nil)))\n"
> > + "(insn 9 8 0 2 (set (reg:SI 79)\n"
> > + " (ashiftrt:SI (subreg:SI (reg:DI 78) 0)\n"
> > + " (const_int 3 [0x3])))\n"
> > + " ../../src/gcc/testsuite/gcc.dg/asr_div1.c:14\n"
> > + " 642 {*aarch64_ashr_sisd_or_int_si3}\n"
> > + " (expr_list:REG_DEAD (reg:DI 78)\n"
> > + " (nil)))\n");
>
> I can sort of see the desire to just copy&paste dumps into this, but
> this strikes me as really ugly. Especially if we end up using real
> targets this hard-codes not just pattern structure but also pattern
> names, which I think is too great a burden on target maintainers.
Note that the loader now resets INSN_CODE to -1, regardless of the
actual code passed in, to force re-recognition, and to isolate the
dumps somewhat from changes to the .md files. So although the above
says insn 641 and 642 (for some snapshot of the aarch64 md file), it
gets reset to -1.
> It's also not unheard of for the insn structure to change a bit; I
> remember a change which swapped location and pattern (I think).
>
> There's also a fairly large amount of visual clutter here, such as
> the
> input filenames.
>
> Maybe there's room for a tool to take input dumps and convert them to
> something more readable, or maybe to a sequence of gen_/emit_
> function
> calls?
(such a tool could use the loader class, and e.g. strip out the
location information).
