Committed, thanks all.
Pan
-----Original Message-----
From: Richard Sandiford <richard.sandif...@arm.com>
Sent: Thursday, November 23, 2023 2:39 AM
To: Li, Pan2 <pan2...@intel.com>
Cc: Richard Biener <richard.guent...@gmail.com>; juzhe.zh...@rivai.ai; Wang,
Yanzhang <yanzhang.w...@intel.com>; kito.ch...@gmail.com; Jeff Law
<jeffreya...@gmail.com>; gcc-patches@gcc.gnu.org
Subject: Re: [PATCH v4] DSE: Allow vector type for get_stored_val when read <
store
"Li, Pan2" <pan2...@intel.com> writes:
>> It looks like Jeff approved the patch?
>
> Yes, just would like to double check the way of this patch is expected as
> following the suggestion of Richard S.
Yeah, it looks good to me, thanks.
Richard
> Pan
>
> -----Original Message-----
> From: Richard Biener <richard.guent...@gmail.com>
> Sent: Wednesday, November 22, 2023 4:02 PM
> To: Li, Pan2 <pan2...@intel.com>
> Cc: richard.sandif...@arm.com; juzhe.zh...@rivai.ai; Wang, Yanzhang
> <yanzhang.w...@intel.com>; kito.ch...@gmail.com; Jeff Law
> <jeffreya...@gmail.com>; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH v4] DSE: Allow vector type for get_stored_val when read <
> store
>
> On Wed, Nov 22, 2023 at 3:30 AM Li, Pan2 <pan2...@intel.com> wrote:
>>
>> Hi Richard S,
>>
>> Thanks a lot for reviewing and comments. May I know is there any concern or
>> further comments for landing this patch to GCC-14?
>
> It looks like Jeff approved the patch?
>
> Richard.
>
>> Pan
>>
>> -----Original Message-----
>> From: Li, Pan2
>> Sent: Wednesday, November 15, 2023 8:25 AM
>> To: gcc-patches@gcc.gnu.org
>> Cc: juzhe.zh...@rivai.ai; Wang, Yanzhang <yanzhang.w...@intel.com>;
>> kito.ch...@gmail.com; richard.guent...@gmail.com; richard.sandif...@arm.com;
>> Jeff Law <jeffreya...@gmail.com>
>> Subject: RE: [PATCH v4] DSE: Allow vector type for get_stored_val when read
>> < store
>>
>> Sorry for disturbing, looks I have a typo for Richard S's email address, cc
>> the right email address for awareness.
>>
>> Pan
>>
>> -----Original Message-----
>> From: Li, Pan2
>> Sent: Wednesday, November 15, 2023 8:18 AM
>> To: Jeff Law <jeffreya...@gmail.com>; gcc-patches@gcc.gnu.org
>> Cc: juzhe.zh...@rivai.ai; Wang, Yanzhang <yanzhang.w...@intel.com>;
>> kito.ch...@gmail.com; richard.guent...@gmail.com; richard.sandiford@arm.com2
>> Subject: RE: [PATCH v4] DSE: Allow vector type for get_stored_val when read
>> < store
>>
>> > I wouldn't try to handle that case unless we had actual evidence it was
>> > useful to do so. Just wanted to point out that unlike pseudos we can
>> > have multiple modes referencing the same memory location.
>>
>> Got the point here, thanks Jeff for emphasizing this, 😉.
>>
>> Pan
>>
>> -----Original Message-----
>> From: Jeff Law <jeffreya...@gmail.com>
>> Sent: Tuesday, November 14, 2023 4:12 AM
>> To: Li, Pan2 <pan2...@intel.com>; gcc-patches@gcc.gnu.org
>> Cc: juzhe.zh...@rivai.ai; Wang, Yanzhang <yanzhang.w...@intel.com>;
>> kito.ch...@gmail.com; richard.guent...@gmail.com; richard.sandiford@arm.com2
>> Subject: Re: [PATCH v4] DSE: Allow vector type for get_stored_val when read
>> < store
>>
>>
>>
>> On 11/12/23 20:22, pan2...@intel.com wrote:
>> > From: Pan Li <pan2...@intel.com>
>> >
>> > Update in v4:
>> > * Merge upstream and removed some independent changes.
>> >
>> > Update in v3:
>> > * Take known_le instead of known_lt for vector size.
>> > * Return NULL_RTX when gap is not equal 0 and not constant.
>> >
>> > Update in v2:
>> > * Move vector type support to get_stored_val.
>> >
>> > Original log:
>> >
>> > This patch would like to allow the vector mode in the
>> > get_stored_val in the DSE. It is valid for the read
>> > rtx if and only if the read bitsize is less than the
>> > stored bitsize.
>> >
>> > Given below example code with
>> > --param=riscv-autovec-preference=fixed-vlmax.
>> >
>> > vuint8m1_t test () {
>> > uint8_t arr[32] = {
>> > 1, 2, 7, 1, 3, 4, 5, 3, 1, 0, 1, 2, 4, 4, 9, 9,
>> > 1, 2, 7, 1, 3, 4, 5, 3, 1, 0, 1, 2, 4, 4, 9, 9,
>> > };
>> >
>> > return __riscv_vle8_v_u8m1(arr, 32);
>> > }
>> >
>> > Before this patch:
>> > test:
>> > lui a5,%hi(.LANCHOR0)
>> > addi sp,sp,-32
>> > addi a5,a5,%lo(.LANCHOR0)
>> > li a3,32
>> > vl2re64.v v2,0(a5)
>> > vsetvli zero,a3,e8,m1,ta,ma
>> > vs2r.v v2,0(sp) <== Unnecessary store to stack
>> > vle8.v v1,0(sp) <== Ditto
>> > vs1r.v v1,0(a0)
>> > addi sp,sp,32
>> > jr ra
>> >
>> > After this patch:
>> > test:
>> > lui a5,%hi(.LANCHOR0)
>> > addi a5,a5,%lo(.LANCHOR0)
>> > li a4,32
>> > addi sp,sp,-32
>> > vsetvli zero,a4,e8,m1,ta,ma
>> > vle8.v v1,0(a5)
>> > vs1r.v v1,0(a0)
>> > addi sp,sp,32
>> > jr ra
>> >
>> > Below tests are passed within this patch:
>> > * The risc-v regression test.
>> > * The x86 bootstrap and regression test.
>> > * The aarch64 regression test.
>> >
>> > PR target/111720
>> >
>> > gcc/ChangeLog:
>> >
>> > * dse.cc (get_stored_val): Allow vector mode if read size is
>> > less than or equal to stored size.
>> >
>> > gcc/testsuite/ChangeLog:
>> >
>> > * gcc.target/riscv/rvv/base/pr111720-0.c: New test.
>> > * gcc.target/riscv/rvv/base/pr111720-1.c: New test.
>> > * gcc.target/riscv/rvv/base/pr111720-10.c: New test.
>> > * gcc.target/riscv/rvv/base/pr111720-2.c: New test.
>> > * gcc.target/riscv/rvv/base/pr111720-3.c: New test.
>> > * gcc.target/riscv/rvv/base/pr111720-4.c: New test.
>> > * gcc.target/riscv/rvv/base/pr111720-5.c: New test.
>> > * gcc.target/riscv/rvv/base/pr111720-6.c: New test.
>> > * gcc.target/riscv/rvv/base/pr111720-7.c: New test.
>> > * gcc.target/riscv/rvv/base/pr111720-8.c: New test.
>> > * gcc.target/riscv/rvv/base/pr111720-9.c: New test.
>> OK for the trunk.
>>
>>
>> >
>>
>> > + else if (VECTOR_MODE_P (read_mode) && VECTOR_MODE_P (store_mode)
>> > + && known_le (GET_MODE_BITSIZE (read_mode), GET_MODE_BITSIZE
>> > (store_mode))
>> > + && targetm.modes_tieable_p (read_mode, store_mode))
>> > + read_reg = gen_lowpart (read_mode, copy_rtx (store_info->rhs));
>> > else
>> > read_reg = extract_low_bits (read_mode, store_mode,
>> > copy_rtx (store_info->rhs));
>> It may not matter, especially for RV, but we could possibly have a
>> mixture of scalar and vector modes in the RTL. Say a vector store
>> followed by a scalar read or vice-versa.
>>
>> I wouldn't try to handle that case unless we had actual evidence it was
>> useful to do so. Just wanted to point out that unlike pseudos we can
>> have multiple modes referencing the same memory location.
>>
>> Jeff
