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
