On 6/15/23 09:30, Manolis Tsamis wrote:
On Thu, Jun 15, 2023 at 6:04 PM Jeff Law <jeffreya...@gmail.com> wrote:
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.
Thanks for reporting. I also noticed this while reworking the
implementation for v2 and I have fixed it among other things.
But I'm still wondering about the type of the offset folding
calculation and whether it could overflow in a bad way:
Could there also be edge cases where HOST_WIDE_INT would be problematic as well?
Maybe unsigned HOST_WIDE_INT is more correct (due to potential overflow issues)?
I think HOST_WIDE_INT is going to be OK. If we overflow a H_W_I, then
there's bigger problems elsewhere.
jeff
