Re: [PATCH] RISC-V/testsuite: Run target testing over all the usual optimization levels

[Vineet Gupta <vineetg@rivosinc.com>]

Hi Thomas,

On 5/25/23 13:56, Thomas Schwinge wrote:
> Hi!
>
> On 2022-02-08T00:22:37+0800, Kito Cheng via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> Hi Maciej:
>>
>> Thanks for doing this, OK to trunk.
>>
>> On Tue, Feb 1, 2022 at 7:04 AM Maciej W. Rozycki <macro@embecosm.com> wrote:
>>> Use `gcc-dg-runtest' test driver rather than `dg-runtest' to run the
>>> RISC-V testsuite as several targets already do.  Adjust test options
>>> across individual test cases accordingly where required.
>>>
>>> As some tests want to be run at `-Og', add a suitable optimization
>>> variant via ADDITIONAL_TORTURE_OPTIONS, and include the moderately
>>> recent `-Oz' variant as well.
>>>
>>>          * testsuite/gcc.target/riscv/riscv.exp: Use `gcc-dg-runtest'
>>>          rather than `dg-runtest'.  Add `-Og -g' and `-Oz' variants via
>>>          ADDITIONAL_TORTURE_OPTIONS.
>>>   As to adding `-Og -g' and `-Oz', this should probably be done globally in
>>> gcc-dg.exp, but such a change would affect all the interested targets at
>>> once and would require a huge one-by-one test case review.  Therefore I
>>> think adding targets one by one instead is more feasible, and then we can
>>> switch once all the targets have.
>>> --- gcc.orig/gcc/testsuite/gcc.target/riscv/riscv.exp
>>> +++ gcc/gcc/testsuite/gcc.target/riscv/riscv.exp
>>> @@ -21,6 +21,8 @@ if ![istarget riscv*-*-*] then {
>>>     return
>>>   }
>>>
>>> +lappend ADDITIONAL_TORTURE_OPTIONS {-Og -g} {-Oz}
>>> +
>>>   # Load support procs.
>>>   load_lib gcc-dg.exp
> Per my understanding, that is not the correct way to do this.  See
> 'gcc/doc/sourcebuild.texi':
>
>      [...] add to the default list by defining
>      @var{ADDITIONAL_TORTURE_OPTIONS}.  Define these in a @file{.dejagnurc}
>      file or add them to the @file{site.exp} file; for example [...]
>
> Notice '.dejagnurc' or 'site.exp', that is: globally.  (Doing this
> "globally in gcc-dg.exp" -- as you'd mentioned above -- would work too,
> conditionalized to '[istarget riscv*-*-*]' only, for now, as you
> suggested.)
>
> Otherwise, per what we've not got, either of the following two happens:
> before any other 'load_lib gcc-dg.exp', 'gcc.target/riscv/riscv.exp'
> happens to be read first, does set 'ADDITIONAL_TORTURE_OPTIONS', then
> does its 'load_lib gcc-dg.exp' -- which then incorporates
> 'ADDITIONAL_TORTURE_OPTIONS' for *all* (!) following '*.exp' files as
> part of that 'runtest' instance.  Alternatively, any other '*.exp' file's
> 'load_lib gcc-dg.exp' comes first (without the desired
> 'ADDITIONAL_TORTURE_OPTIONS' being set), and once
> 'gcc.target/riscv/riscv.exp' is read, while it then does set
> 'ADDITIONAL_TORTURE_OPTIONS', its 'load_lib gcc-dg.exp' is a no-op, as
> that one has already been loaded, and therefore the
> 'ADDITIONAL_TORTURE_OPTIONS' aren't incorporated.
>
> Instead, I suggest to do this locally: do 'load_lib torture-options.exp',
> 'torture-init', 'set-torture-options [...]' (where that includes your
> special options), 'gcc-dg-runtest', 'torture-finish'.  See other '*.exp'
> files.
>
>
> (No, I didn't invent this interface.)
>
>
> (I however don't see yet how this would be related to the current
> "ERROR: torture-init: torture_without_loops is not empty as expected"
> discussion, as has, kind of, been claimed in
> <https://inbox.sourceware.org/d7e7dcfa-6ef7-959b-05d8-fb742f281554@rivosinc.com>
> "RISC-V: Add missing torture-init and torture-finish for rvv.exp" and the
> following.)

Thanks for taking a look at this. Please don't get me wrong, never mean 
to vilify the patches above - and I should have verified first (by 
reverting those) if they caused the issue - if at all. It just seemed 
that we started seeing these relatively recently and the timing of your 
changes seemed to coincide. As you say above, RV likely has existing 
less than ideal constructs which somehow used to work before, but not in 
the new regime.

Anyhow the goal is to get this fixed for RV and any more help will be 
appreciated since I'm really a TCL noob.

It seems my claim yesterday that adding torture-{init,finish} fixed RV 
issues were just premature. I was trying a mix of running the full suite 
vs. just  RUNTESTFLAGS="riscv.exp" and in some cases latter can give a 
false positive (I was making sure dejagnu got rebuilt and rekicked etc, 
but anyhow different issue).

I'm currently removing the ADDITIONAL_TORTURE_OPTIONS to see if this 
helps cure it and then try the new sequence you pointed to above.

FWIW if you want to test this out at your end, it is super easy.

|git clone https://github.com/riscv-collab/riscv-gnu-toolchain 
toolchain-upstream cd toolchain-upstream git submodule init git 
submodule update ||./configure --with-arch=rv64imafdc --with-abi=lp64d --enable-multilib 
--enable-linux --prefix=<path-to-install> make -j make report-linux 
SIM=qemu Thx, -Vineet |||