public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Robin Dapp <rdapp.gcc@gmail.com>
To: Joern Rennecke <joern.rennecke@embecosm.com>
Cc: rdapp.gcc@gmail.com, GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: RISCV test infrastructure for d / v / zfh extensions
Date: Mon, 21 Aug 2023 22:24:03 +0200	[thread overview]
Message-ID: <30086163-c4d4-64c2-b4cb-9cf2c866678c@gmail.com> (raw)
In-Reply-To: <CAMqJFCrOtzmwKmV0T6EuK1t=1C0N5AonFOAAk6vjyUQsYGomkw@mail.gmail.com>

Hi Joern.

> Hmm, you are right.  I personally prefer my version because it allows
> consistent naming of the
> different tests, also easily extendible when new extensions need testing.
> Although the riscv_vector name has the advantage that it is better
> legible for people who are
> not used to dealing with RISC_V extension names.  If we keep
> riscv_vector, it would make
> sense to name the other tests also something more verbose, e.g. change
> riscv_d into
> riscv_double_fp or even riscv_double_precision_floating_point .
> It would be nice to hear other people's opinions on the naming.

I can live with either with a preference for your naming scheme, i.e. 
calling the extensions directly by their name for consistency reasons.
A more verbose scheme might lead to misconceptions later in case we
have several closely related extensions.  There will probably already be
ample discussion during ratification about naming and IMHO we should
not repeat that just to make names more accessible.  If needed we can
still add comments in the respective tests to clarify.
Vector is usually special among architecture extensions but we're not
even consistent with naming in the source itself, so...  

>> Would it make sense to skip the first check here
>> (check_effective_target_riscv_v) so we have a proper runtime check?
> 
> My starting point was that the changing of global testsuite variables around -
> as the original RISC-V vector patches did - is wrong.  The user asked to test
> a particular target (or set targets, for multilibs), and that target
> is the one to test,
> so we can't just assume it has other hardware features that are not implied by
> the target.
> Contrarily, the target that the user requested to test can be assumed to be
> available for testing.  Testing that it actually works is a part of
> the point of the
> test.  If I ask for a dejagnu test for a target that has vector support, I would
> hope that the vector support is also tested, not backing off if it finds that
> there is a problem with the target,
> The way I look at things, when the macro  __riscv_v is defined,
> the compiler asserts that it is compiling for a target that has vector support,
> because it was instructed by configuration / options to emit code for that
> target.  Which we can take as evidence that dejagnu is run with options
> to select that target (either explicitly or by default due to the
> configuration of
> the compiler under test)

Yes, I largely agree with that.  Where I was coming from is that several other
effective target checks will not short circuit the check but always perform it
fully (i.e. interpreting the effective target as the full chain up to execution).
Yet, I can see the appeal of the short circuit as well and in the end it really
doesn't matter all that much.

I would have preferred to replace the existing checks right away in order to
immediately have proper coverage but let's not dwell on that, therefore
LGTM, thanks. 

Regards
 Robin

      reply	other threads:[~2023-08-21 20:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-18  6:02 Joern Rennecke
2023-08-01 13:44 ` Robin Dapp
2023-08-15  1:24   ` Joern Rennecke
2023-08-21 20:24     ` Robin Dapp [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=30086163-c4d4-64c2-b4cb-9cf2c866678c@gmail.com \
    --to=rdapp.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=joern.rennecke@embecosm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).