On 2/5/25 11:11 AM, Palmer Dabbelt wrote:
On Wed, 05 Feb 2025 09:57:56 PST (-0800), ja...@redhat.com wrote:
Hi!

The following test ICEs on RISC-V at least latently since
r14-1622-g99bfdb072e67fa3fe294d86b4b2a9f686f8d9705 which added
RISC-V specific case to get_biv_step_1 to recognize also
({zero,sign}_extend:DI (plus:SI op0 op1))

The reason for the ICE is that op1 in this case is CONST_POLY_INT
which unlike the really expected VOIDmode CONST_INTs has its own
mode and still satisfies CONSTANT_P.
GET_MODE (rhs) (SImode) is different from outer_mode (DImode), so
the function later does
        *inner_step = simplify_gen_binary (code, outer_mode,
                                           *inner_step, op1);
but that obviously ICEs because while *inner_step is either VOIDmode
or DImode, op1 has SImode.

The following patch fixes it by extending op1 using code so that
simplify_gen_binary can handle it.  Another option would be
to change the !CONSTANT_P (op1) 3 lines above this to
!CONST_INT_P (op1), I think it isn't very likely that we get something
useful from other constants there.

Bootstrapped/regtested on x86_64-linux and i686-linux (which doesn't
tell much because such code isn't encountered there, just that the
gcc.dg test at least works), but I have no way to test this on riscv
easily.  Could somebody test it there?

The precommit CI should pick it up, as it's got "RISC-V" in the subject line.  I don't see it in the list yet, though.
It picked it up, but Jakub strips a component off the pathnames in his patches (the leading a/b) which confused the pre-commit CI tester.

Regardless I can spin it in my own.


Or do you want the
!CONST_INT_P (op1) instead?

Jeff would know way better than I do here, but I think this is just trying to match addiw-type patterns and thus CONST_INT would be OK because we only have 12-bit constants here.
My recollection of the last touch of this code was it helps us "see through" the extension that commonly appears in risc-v code. It addressed a relatively minor regression Jivan saw when changing the various risc-v expanders to expose the implicit sign extension.

Jeff

Reply via email to