LGTM
On Sun, Nov 19, 2023 at 1:36 PM Maciej W. Rozycki <ma...@embecosm.com> wrote: > > For the NEED_EQ_NE_P `riscv_emit_int_compare' is documented to only emit > EQ or NE comparisons against zero, however it does not catch incorrect > use where a non-equality comparison has been requested and falls through > to the general case then. Add a safety guard to catch such a case then. > > Arguably the NEED_EQ_NE_P case would best be moved into a function of > its own, but let's leave it for a separate cleanup. > > gcc/ > * config/riscv/riscv.cc (riscv_emit_int_compare): Bail out if > NEED_EQ_NE_P but the comparison is neither EQ nor NE. > --- > FWIW the structure of code here clearly shows the NEED_EQ_NE_P case has > been bolted on as an afterthought rather than how this piece would look > if written from scratch right away. Let's defer any further cleanups at > this stage of the development cycle though. > --- > gcc/config/riscv/riscv.cc | 1 + > 1 file changed, 1 insertion(+) > > gcc-riscv-emit-int-compare-need-eq-ne.diff > Index: gcc/gcc/config/riscv/riscv.cc > =================================================================== > --- gcc.orig/gcc/config/riscv/riscv.cc > +++ gcc/gcc/config/riscv/riscv.cc > @@ -3779,6 +3779,7 @@ riscv_emit_int_compare (enum rtx_code *c > *op1 = const0_rtx; > return; > } > + gcc_unreachable (); > } > > if (splittable_const_int_operand (*op1, VOIDmode))
