On 9/29/26 8:47 AM, wangjue wrote:
Hi Jeff

Wangjue -- there's nothing inherently wrong with the testcase
adjustments, but we prefer to have at least some understanding of why
the test was changed.
If the code vectorized before with the cost model, but doesn't vectorize
after your change with the default cost model *and* the code is likely
profitable to vectorize, then you're introducing a code quality regression.
If the code isn't likely to be profitable with vectors, then your change
is good and exhibits the behavior we'd want to see.
So that's really the key thing we want to understand -- is the testsuite
change covering up a code quality regression or is it the case that the
code isn't profitable to vectorize and thus turning off the cost model
to force vectorization is the right thing to do for the test.
These tests check SAT_TRUNC recognition and the expected vnclipu instructions. 
I added -fno-vect-cost-model because a downstream build configured with 
--with-tune=xt-c950 rejected both loops. The intent was to keep the instruction 
checks independent of tuning, though this also reduces coverage of 
vectorization decisions under the default cost model.
Ah, yes.  This is a common issue.  And yes, if we can get the test to be independent of tuning, then that is and of itself a good thing.

I rechecked both tests on the same upstream baseline, with and without the 
patch. Without disabling the cost model, both vectorize and produce identical 
assembly. The option changes are therefore unnecessary for the upstream 
configurations checked.

Would you prefer that I drop these two option changes and keep the original 
test options?
If they bring more stability to the results if someone was to configure using --with-cpu, --with-arch or --with-tune, then let's go ahead and commit them.  The other approach is to use explicit cpu, arch or tune arguments in the test itself, but I suspect what you've done is just as effective.

Thanks for double checking.  The testsuite bits are OK for RISC-V.

jeff

Reply via email to