Re: [PATCH 0/2] RISC-V: New pass to optimize calculation of offsets for memory operations.

[Jeff Law <jeffreyalaw@gmail.com>]

On 5/25/23 07:42, Jeff Law wrote:

> Thanks Manolis.  Do you happen to know if this includes the fixes I 
> passed along to Philipp a few months back?  My recollection is one fixed 
> stale DF data which prevented an ICE during bootstrapping, the other 
> needed to ignore debug insns in one or two places so that the behavior 
> didn't change based on the existence of debug insns.
So we stumbled over another relatively minor issue in this code this 
week that I'm sure you'll want to fix for a V2.

Specifically fold_offset's "scale" argument needs to be a HOST_WIDE_INT 
rather than an "int".  Inside the ASHIFT handling you need to change the 
type of shift_scale to a HOST_WIDE_INT as well and potentially the 
actual computation of shift_scale.

The problem is if you have a compile-time constant address on rv64, it 
might be constructed with code like this:




> (insn 282 47 283 6 (set (reg:DI 14 a4 [267])
>         (const_int 348160 [0x55000])) "test_dbmd_pucinterruptenable_rw.c":18:31 179 {*movdi_64bit}
>      (nil))
> (insn 283 282 284 6 (set (reg:DI 14 a4 [267])
>         (plus:DI (reg:DI 14 a4 [267])
>             (const_int 1365 [0x555]))) "test_dbmd_pucinterruptenable_rw.c":18:31 5 {riscv_adddi3}
>      (expr_list:REG_EQUAL (const_int 349525 [0x55555])
>         (nil)))
> (insn 284 283 285 6 (set (reg:DI 13 a3 [268])
>         (const_int 1431662592 [0x55557000])) "test_dbmd_pucinterruptenable_rw.c":18:31 179 {*movdi_64bit}
>      (nil))
> (insn 285 284 215 6 (set (reg:DI 13 a3 [268])
>         (plus:DI (reg:DI 13 a3 [268])
>             (const_int 4 [0x4]))) "test_dbmd_pucinterruptenable_rw.c":18:31 5 {riscv_adddi3}
>      (expr_list:REG_EQUAL (const_int 1431662596 [0x55557004])
>         (nil)))
> (insn 215 285 216 6 (set (reg:DI 14 a4 [271])
>         (ashift:DI (reg:DI 14 a4 [267]) 
>             (const_int 32 [0x20]))) "test_dbmd_pucinterruptenable_rw.c":18:31 204 {ashldi3}
>      (nil)) 
> (insn 216 215 42 6 (set (reg/f:DI 14 a4 [166])
>         (plus:DI (reg:DI 14 a4 [271]) 
>             (reg:DI 13 a3 [268]))) "test_dbmd_pucinterruptenable_rw.c":18:31 5 {riscv_adddi3}
>      (expr_list:REG_DEAD (reg:DI 13 a3 [268])
>         (expr_list:REG_EQUIV (const_int 1501199875796996 [0x5555555557004])
>             (nil))))



Note that 32bit ASHIFT in insn 215.  If you're doing that computation in 
a 32bit integer type, then it's going to shift off the end of the type.


Jeff