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

[Thomas Schwinge <thomas.schwinge@siemens.com>]

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.)


Grüße
 Thomas


>> @@ -34,7 +36,7 @@ if ![info exists DEFAULT_CFLAGS] then {
>>  dg-init
>>
>>  # Main loop.
>> -dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]] \
>> +gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]] \
>>         "" $DEFAULT_CFLAGS
>>
>>  # All done.