Re: [r13-7135 Regression] FAIL: gcc.dg/vect/vect-simd-clone-18f.c scan-tree-dump-times vect "[\\n\\r] [^\\n]* = foo\\.simdclone" 2 on Linux/x86_64

["Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com>]

On 13/04/2023 11:01, Andrew Stubbs wrote:
> Hi Andre,
> 
> I don't have a cascadelake device to test on, nor any knowledge about 
> what makes it different from regular x86_64.

Not sure you need one, but yeah I don't know either, it looks like it 
fails because:
in-branch vector clones are not yet supported for integer mask modes.

A quick look tells me this is because mask_mode is not VOIDmode. 
i386.cc's TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN will set 
mask_mode to either DI or SI mode when TARGET_AVX512F. So I suspect 
cascadelake is TARGET_AVX512F.

This is where I bail out as I really don't want to dive into the target 
specific simd clone handling of x86 ;)

> 
> If the cascadelake device is supposed to work the same as other x86_64 
> devices for these vectors then the test has found a bug in the compiler 
> and you should be looking to fix that, not fudge the testcase.
> 
> Alternatively, if the device's capabilities really are different and the 
> tests should behave differently, then the actual expectations need to be 
> encoded in the dejagnu directives. If you can't tell the difference by 
> looking at the "x86_64*-*-*" target selector alone then the correct 
> solution is to invent a new "effective-target" selector. There are lots 
> of examples of using these throughout the testsuite (you could use 
> dg-require-effective-target to disable the whole testcase, or just use 
> the name in the scan-tree-dump-times directive to customise the 
> expectations), and the definitions can be found in the 
> lib/target-supports.exp and lib/target-supports-dg.exp scripts. Some are 
> fixed expressions and some run the compiler to probe the configuration, 
> but in this case you probably want to do something with "check-flags".

Even though I agree with you, I'm not the right person to do this 
digging for such target specific stuff. So for now I'd probably suggest 
xfailing this for avx512f.
> 
> For the unroll problem, you can probably tweak the optimization options 
> to disable that, the same as has been done for the epilogues feature 
> that had the same problem.

I mistaken the current behaviour for unrolling, it's actually because of 
a latent bug. The vectorizer calls `vect_get_smallest_scalar_type` to 
determine the vectype of a stmt. For a function like foo, that has the 
same type (long long) everywhere this wouldn't be a problem, however, 
because you transformed it into a MASK_CALL that has a function pointer 
(which is 32-bit in -m32) that now becomes the 'smallest' type.

This is all a red-herring though, I don't think we should be calling 
this function for potential simdclone calls as the type on which the 
veclen is not necessarily the 'smallest' type. And some arguments (like 
uniform and linear) should be ignored anyway as they won't be mapped to 
vectors.  So I do think this might have been broken even before your 
changes, but needs further investigation.
> Since these are new tests for a new feature, I don't really understand 
> why this is classed as a regression?
> 
> Andrew
> 
> P.S. there was a commit to these tests in the last few days, so make 
> sure you pull that before making changes.

The latest commit to these tests was mine, it's the one Haochen is 
reporting this regression against. My commit was to fix the issue richi 
had introduced that was preventing the feature you introduced from 
working. The reason nobody noticed was because the tests you introduced 
didn't actually test your feature, since you didn't specify 'inbranch' 
the omp declare simd pragma was allowing the use of not-inbranch simd 
clones and the vectorizer was being smart enough to circumvent the 
conditional and was still able to use simdclones (non inbranch ones) so 
when the inbranch stopped working, the test didn't notice.

The other changes to this test were already after the fix for 108888 
that broke the inbranch feature you added, and so it was fixing a 
cascadelake testism but for the not-inbranch simdclones. So basically 
fixing a testism of a testism :/


I am working on simdclone's for AArch64 for next Stage 1 so I don't mind 
looking at the issue with the vectype being chosen wrongly, as for the 
other x86 specific testisms I'll leave them to someone else.

Kind Regards,
Andre