On 11/18/24 11:09 AM, Patrick O'Neill wrote:
On 11/18/24 09:57, Jeff Law wrote:
And stage3 begins...
Zdenek's fuzzer caught this one. Essentially using
simplify_gen_subreg directly with an offset of 0 when we just needed a
lowpart.
The offset of 0 works for little endian, but for big endian it's
simply wrong. simplify_gen_subreg will return NULL_RTX because the
case isn't representable. We then embed that NULL_RTX into an insn
that's later scanned during mark_jump_label.
Scanning the port I see a couple more instances of this incorrect
idiom. One is pretty obvious to fix. The others look a bit goofy
and I'll probably need to sync with Patrick on them.
Anyway tested on riscv64-elf and riscv32-elf with no regressions.
Pushing to the trunk.
jeff
Thanks! Also in the big-endian space:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109933
I have a patch attached to that bug but never landed it since I didn't
have a way to test it.
Yea, that happens sometimes, we muddle through as best as we can.
Sometimes we have to acknowledge some things we can't reasonably test
and we do the best we can.
I was actually referring to something different though.
If we look at riscv_lshift_subword we have:
emit_move_insn (value_reg, simplify_gen_subreg (SImode, value,
mode, 0));
That's going to mis-behave on a big endian target. The only uses are
from sync.md.
The intent of the code was a bit unclear. So knowing what that was
supposed to be doing would help a lot. If the intent was to look at
VALUE in SImode, then can't we just use value_reg = force_reg (SImode,
value)? But I couldn't convince myself that was the actual intent.
Jeff
