Re: [PATCH 1/3] testsuite: Unbork multilib testing on RISC-V (and any target really)

[Thomas Schwinge <thomas@codesourcery.com>]

Hi!

On 2023-06-01T09:24:20+0200, I wrote:
> First, Vineet, great that you've now tracked this down!  :-) Indeed
> "early exit" vs. 'torture-finish' was exactly the issue that I suspected.
>
> It may not be what you originally intended, but I hope at least you've
> learned some things about DejaGnu/TCL...  ;-P
>
> Yesterday, I actually had begun looking into this.  To avoid the big
> download and having to wait for a lot of packages to be build with your
> 'riscv-gnu-toolchain' recipe:
> <https://inbox.sourceware.org/44506218-70bd-0b7b-a560-744bb24376a9@rivosinc.com>,
> I intended to do just a quick GCC build on compile farm gcc92, which
> (a) didn't turn out to be quick, and (b) eventually failed due to
> <https://gcc.gnu.org/PR106271>
> "Bootstrap on RISC-V on Ubuntu 22.04 LTS: bits/libc-header-start.h: No such file or directory"...
>
> (I'm now running 'riscv-gnu-toolchain' to verify this, and another thing.)

If running that with just 'RUNTESTFLAGS="i386-prefetch.exp"', I get:

    [...]
    Schedule of variations:
        riscv-sim/-march=rv32imac/-mabi=ilp32/-mcmodel=medlow
        riscv-sim/-march=rv32imafdc/-mabi=ilp32d/-mcmodel=medlow
        riscv-sim/-march=rv64imac/-mabi=lp64/-mcmodel=medlow
        riscv-sim/-march=rv64imafdc/-mabi=lp64d/-mcmodel=medlow

    Running target riscv-sim/-march=rv32imac/-mabi=ilp32/-mcmodel=medlow
    [...]
    Running [...]/gcc.misc-tests/i386-prefetch.exp ...

                    === gcc Summary for riscv-sim/-march=rv32imac/-mabi=ilp32/-mcmodel=medlow ===

    Running target riscv-sim/-march=rv32imafdc/-mabi=ilp32d/-mcmodel=medlow
    [...]
    Running [...]/gcc.misc-tests/i386-prefetch.exp ...
    ERROR: tcl error sourcing [...]/gcc.misc-tests/i386-prefetch.exp.
    ERROR: tcl error code NONE
    ERROR: torture-init: LTO_TORTURE_OPTIONS is not empty as expected
    [...]

..., which indeed complains about 'LTO_TORTURE_OPTIONS' (which relates to
my recent changes in that area, which I now do understand, see below).

The issue is that indeed 'torture-init' now does set
'LTO_TORTURE_OPTIONS', whereas 'torture_without_loops',
'torture_with_loops' are set only when 'set-torture-options' is called.

> Before we push your patch, let me please verify that it indeed doesn't
> change any 'gcc.misc-tests/i386-prefetch.exp' semantics

Done.

> and:
>
> On 2023-05-31T19:13:01+0100, Iain Sandoe via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>> On 31 May 2023, at 18:57, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>> On 5/31/23 10:25, Vineet Gupta wrote:
>>>> Multilib testing on trunk is currently busted (and surprisingly this
>>>> affects any/all targets but it seems nobody cares). We currently get the
>>>> following splat:
>>> I wouldn't say that nobody cares, it just hasn't bubbled up on anyone's priority list yet (most developers aren't working on targets that make heavy use of multilibs).
>
> So I regularely do build x86_64 GNU/Linux with default '-m64' plus '-m32'
> multilib -- but of course, there's no "early exit" for those, as there's
> no 'string match "* -march=*" " [board_info target multilib_flags] "'...
>
>>> But probably more importantly, this problem seems to not be triggering on all multilib targets.  For example, I just examined my tester's build logs and couldn't see this on the H8/300 or V850 ports.  Which begs the question, why?
>
> ..., which may be the case for those, too?  In other words: the problem
> only shows up if '-march=[...]' appears in the flags, which indeed may
> not be a common thing?  I'll cross-verify this with x86_64 and
> '-march=[...]' flags.

That is the crucial thing indeed.  Vineet, please note that in the Git
commit log.  That is, instead of "Multilib testing", say "Multilib
testing involving '-march=[...]' flags", or similar.

The ERRORs do reproduce with x86_64 GNU/Linux with:

    RUNTESTFLAGS='--target_board=unix\{-m32,-m64,-march=generic,-march=generic\} i386-prefetch.exp'

..., for example.  Here, '-m32' behaves as expected, '-m64' behaves as
expected, the first '-march=generic' does the 'torture-init' and "early
exit", the second '-march=generic' then again does 'torture-init' and
runs into the error condition.

> And, I still intend to figure out why this issue apparently disappears
> with my recent 'LTO_TORTURE_OPTIONS' patches reverted:
> <https://inbox.sourceware.org/ad8a98da-0231-7a95-017b-ea5d8ae65678@rivosinc.com>.

In the "old world", 'torture-init', *not* followed by
'set-torture-options', *not* followed by 'torture-finish', then another
'torture-init' was not a problem -- but in the "new world" it now is.

This also explains my confusion; the original report was:

    ERROR: torture-init: torture_without_loops is not empty as expected

..., note: not 'LTO_TORTURE_OPTIONS' but 'torture_without_loops', and
those I'd not directly touched in my recent changes, which had made me
confused.

The 'torture_without_loops' error condition now does arise if there's a
'torture-init', *not* followed by 'set-torture-options', *not* followed
by 'torture-finish' (that is, 'i386-prefetch.exp'), then 'gcc-dg-runtest'
('riscv.exp', for example), which internally skips 'torture-init' (due to
'torture-init-done', due to 'LTO_TORTURE_OPTIONS'), but it does
'set-torture-options', then skips 'torture-finish', and then any next
'torture-init' detects the mismatch, thus ERRORs.


I can be convinced otherwise, but I still maintain my position that
requiring/checking proper bracketing of 'torture-init', 'torture-finish'
is advisable, and therefore propose to not re-do my 'LTO_TORTURE_OPTIONS'
changes, but indeed suggest to apply Vineet's patch, with a minor change,
see below.  (I cannot formally approve it, however; testsuite maintainers
CCed.)


As for your proposed patch:
<https://inbox.sourceware.org/20230531162534.119952-2-vineetg@rivosinc.com>,
I suggest to move the "early exit" in front of all the setup code, that
is, right after license header:

    [...]
    # <http://www.gnu.org/licenses/>.

    [HERE]

    # Test that the correct data prefetch instructions (SSE or 3DNow! variant,
    [...]


Grüße
 Thomas


> Otherwise:
>
>> I do have a multilib problem [with libgomp] on Darwin (which has been noticed : https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109951) but it is not obvious how the fix proposed would solve this - unless it’s some subtle change in global content for the multilib options.
>>
>> (testing anyway)
>
> No, this is really a separate issue.  I understand what's happening, and
> have an idea about how to address this that I'll post later.
>
>
> Grüße
>  Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955