Re: Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv.
>> Yeah, just noticed that myself. Anyway will do some more tests, >> maybe my initial VLS analysis was somehow flawed. You can check binop_vx_constraint-167.c ~ binop_vx_constraint-174.c This patch is pre-approved if you change as my suggestion. I am gonna sleep so I am not able to review again. Feel free to commit it after change as I suggested. Thanks. juzhe.zh...@rivai.ai From: Robin Dapp Date: 2023-11-17 23:13 To: 钟居哲; gcc-patches; palmer; kito.cheng; Jeff Law CC: rdapp.gcc Subject: Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv. > It must be correct. We already have test (intrinsic codes) for it. Yeah, just noticed that myself. Anyway will do some more tests, maybe my initial VLS analysis was somehow flawed. > Condition should be put into iterators (Add a new iterator for > indexed load store). Ah, that's what you meant. Sure. Regards Robin
Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv.
> It must be correct. We already have test (intrinsic codes) for it. Yeah, just noticed that myself. Anyway will do some more tests, maybe my initial VLS analysis was somehow flawed. > Condition should be put into iterators (Add a new iterator for > indexed load store). Ah, that's what you meant. Sure. Regards Robin
Re: Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv.
>> I'm wondering whether the VLA modes in the iterator are correct. >> Looks dubious to me but unsure, will need to create some tests >> before continuing. It must be correct. We already have test (intrinsic codes) for it. >> What's the problem with those? We probably won't reach there >> because the indexed is considered invalid before but we could, >> theoretically, still combine them? Condition should be put into iterators (Add a new iterator for indexed load store). Like you did on RAITO iterators adding !TARGET_64BIT. It's easier to maintain since iterators codes only happence once. Wheras this patch is adding GET_MODE_BITSIZE (GET_MODE_INNER (mode)) <= GET_MODE_BITSIZE (Pmode) in many places. juzhe.zh...@rivai.ai From: Robin Dapp Date: 2023-11-17 22:55 To: 钟居哲; gcc-patches; palmer; kito.cheng; Jeff Law CC: rdapp.gcc Subject: Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv. > OK. Make sense。 I'm wondering whether the VLA modes in the iterator are correct. Looks dubious to me but unsure, will need to create some tests before continuing. > LGTM as long as you remove all > GET_MODE_BITSIZE (GET_MODE_INNER (mode)) <= GET_MODE_BITSIZE (Pmode) What's the problem with those? We probably won't reach there because the indexed is considered invalid before but we could, theoretically, still combine them? Regards Robin
Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv.
> OK. Make sense。 I'm wondering whether the VLA modes in the iterator are correct. Looks dubious to me but unsure, will need to create some tests before continuing. > LGTM as long as you remove all > GET_MODE_BITSIZE (GET_MODE_INNER (mode)) <= GET_MODE_BITSIZE (Pmode) What's the problem with those? We probably won't reach there because the indexed is considered invalid before but we could, theoretically, still combine them? Regards Robin
Re: Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv.
OK. Make sense。 LGTM as long as you remove all GET_MODE_BITSIZE (GET_MODE_INNER (mode)) <= GET_MODE_BITSIZE (Pmode) juzhe.zh...@rivai.ai From: Robin Dapp Date: 2023-11-16 04:30 To: 钟居哲; gcc-patches; palmer; kito.cheng; Jeff Law CC: rdapp.gcc Subject: Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv. On 11/15/23 15:29, 钟居哲 wrote: > Could you show me the example ? > > It's used by handling SEW = 64 on RV32. I don't know why this patch touch > this code. Use gather_load_run-1.c with the 64-bit index patterns disabled on rv32. We insert (mem:DI (reg:SI)) into a vector so use the SEW = 64 demote handler. There we set vl = vl * 2 (which is correct) but the mode (i.e. vector) just changes from DI to SI while keeping the number of elements the same. Then we get e.g. go from V8DI to V8SI and slide down 16 elements, losing the lower half. Regards Robin
Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv.
On 11/15/23 15:29, 钟居哲 wrote: > Could you show me the example ? > > It's used by handling SEW = 64 on RV32. I don't know why this patch touch > this code. Use gather_load_run-1.c with the 64-bit index patterns disabled on rv32. We insert (mem:DI (reg:SI)) into a vector so use the SEW = 64 demote handler. There we set vl = vl * 2 (which is correct) but the mode (i.e. vector) just changes from DI to SI while keeping the number of elements the same. Then we get e.g. go from V8DI to V8SI and slide down 16 elements, losing the lower half. Regards Robin
Re: Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv.
Could you show me the example ? It's used by handling SEW = 64 on RV32. I don't know why this patch touch this code. juzhe.zh...@rivai.ai From: Robin Dapp Date: 2023-11-15 22:27 To: 钟居哲; gcc-patches; palmer; kito.cheng; Jeff Law CC: rdapp.gcc Subject: Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv. > Looks wrong. Recover back. When we demote we use two elements where there was one before. Therefore the vector needs to be able to hold twice as many elements. We adjust vl correctly but the mode is not here. Regards Robin
Re: [PATCH] RISC-V: Disallow 64-bit indexed loads and stores for rv32gcv.
> Looks wrong. Recover back. When we demote we use two elements where there was one before. Therefore the vector needs to be able to hold twice as many elements. We adjust vl correctly but the mode is not here. Regards Robin
