Thanks Jeff for comments. > So if this change is the right one to make for the strided subdirectory, > then shouldn't it also be correct to make for the gather-scatter > subdirectory as well?
> And similarly for various other instances where we call dg-runtest in > that file. Yes, all "" "$op" of rvv.exp need to change to "$op" "" if we would like to test sorts of rvv option combinations. > Basically I'd like to see some explanation why this is the right patch > to make and why this case needs to be handled different from every other > one that I see in that file. Assuming that explanation makes sense, > then some kind of comment i this file indicating why this case is > different seems in order. But if we make all those changes together, there will be lots of increased failure cases. Thus, I prefer to fix it one by one (like strided, then gather ... etc), to make sure the patch could be friendly for review, as well as avoid any new failures of rvv.exp anonymously. Is there any best practice for such kind of changes ? Pan -----Original Message----- From: Jeff Law <jeffreya...@gmail.com> Sent: Tuesday, November 19, 2024 10:04 PM To: Li, Pan2 <pan2...@intel.com>; gcc-patches@gcc.gnu.org Cc: juzhe.zh...@rivai.ai; kito.ch...@gmail.com; rdapp....@gmail.com Subject: Re: [PATCH v1 1/2] RISC-V: Fix incorrect optimization options passing to strided ld/st test On 11/19/24 1:30 AM, pan2...@intel.com wrote: > From: Pan Li <pan2...@intel.com> > > The testcases of vector strided load/store are designed to pick up > different sorts of optimization options but actually these option > are ignored according to the Execution log of gcc.log. This patch > would like to make it correct, and then you will see the build option > similar as below from the gcc.log. > > Executing ... strided_ld_st-1-f16.c -O3 -mrvv-vector-bits=scalable > -mrvv-max-lmul=m1 ... > Executing ... strided_ld_st-1-f16.c -O3 -mrvv-vector-bits=zvl > -mrvv-max-lmul=m1 ... > Executing ... strided_ld_st-1-f16.c -O3 -mrvv-vector-bits=scalable > -mrvv-max-lmul=m4 ... > Executing ... strided_ld_st-1-f16.c -O3 -mrvv-vector-bits=scalable > -mrvv-max-lmul=m8 ... > Executing ... strided_ld_st-1-f16.c -O3 -mrvv-vector-bits=zvl > -mrvv-max-lmul=dynamic ... > Executing ... strided_ld_st-1-f16.c -O3 -mrvv-vector-bits=zvl > -mrvv-max-lmul=m8 ... > Executing ... strided_ld_st-1-f16.c -O3 -mrvv-vector-bits=zvl > -mrvv-max-lmul=m4 ... > Executing ... strided_ld_st-1-f16.c -O3 -mrvv-vector-bits=scalable > -mrvv-max-lmul=m2 ... > Executing ... strided_ld_st-1-f16.c -O3 -mrvv-vector-bits=scalable > -mrvv-max-lmul=dynamic ... > Executing ... strided_ld_st-1-f16.c -O3 -mrvv-vector-bits=zvl > -mrvv-max-lmul=m2 ... > > The below test suites are passed for this patch. > * The rv64gcv fully regression test. > > It is test only patch and obvious up to a point, will commit it > directly if no comments in next 48H. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/rvv/rvv.exp: Fix the incorrect optimization options. So if this change is the right one to make for the strided subdirectory, then shouldn't it also be correct to make for the gather-scatter subdirectory as well? And similarly for various other instances where we call dg-runtest in that file. Basically I'd like to see some explanation why this is the right patch to make and why this case needs to be handled different from every other one that I see in that file. Assuming that explanation makes sense, then some kind of comment i this file indicating why this case is different seems in order. jeff
