On 7/5/24 1:28 AM, Sébastien Michelland wrote:
Hi Oleg!
I don't understand why this is being limited to SH3 and SH4 only?
Almost all SH4 systems out there have an FPU (unless special
configurations
are used). So I'd say if switching to soft-fp, then for SH-anything, not
just SH3/SH4.
If it yields some improvements for some users, I'm all for it.
Yeah I just defaulted to SH3/SH4 conservatively because that's the only
hardware I have. (My main platform also happens to be one of these SH4
without an FPU, the SH4AL-DSP.)
Once this is tested/validated on simulator, I'll happily simplify the
patch to apply to all SH.
I think it would make sense to test it using sh-sim on SH2 big-endian and
little endian at least, as that doesn't have an FPU and hence would run
tests utilizing soft-fp.
After building the toolchain for --target=sh-elf, you can use this to run
the testsuite in the simulator:
make -k check RUNTESTFLAGS="--target_board=sh-sim\{-m2/-ml,-m2/-mb}"
(add make -j parameter according to you needs -- it will be slow)
Alright, it might take a little bit.
Building the combined tree of gcc/binutils/newlib masters (again
following [1]) gives me an ICE in libstdc++v3/src/libbacktrace,
irrespective of my libgcc change:
This is almost certainly a poorly written pattern. I just fixed a bunch
of these, but not this one. Essentially a recent change in the generic
parts of the compiler is exposing some bugs in the SH backend.
Specifically:
;; Store (negated) T bit as all zeros or ones in a reg.
;; subc Rn,Rn ! Rn = Rn - Rn - T; T = T
;; not Rn,Rn ! Rn = 0 - Rn
;;
;; Note the call to sh_split_treg_set_expr may clobber
;; the T reg. We must express this, even though it's
;; not immediately obvious this pattern changes the
;; T register.
(define_insn_and_split "mov_neg_si_t"
[(set (match_operand:SI 0 "arith_reg_dest" "=r")
(neg:SI (match_operand 1 "treg_set_expr")))
(clobber (reg:SI T_REG))]
"TARGET_SH1"
{
gcc_assert (t_reg_operand (operands[1], VOIDmode));
return "subc %0,%0";
}
"&& can_create_pseudo_p () && !t_reg_operand (operands[1], VOIDmode)"
[(const_int 0)]
{
sh_treg_insns ti = sh_split_treg_set_expr (operands[1], curr_insn);
emit_insn (gen_mov_neg_si_t (operands[0], get_t_reg_rtx ()));
if (ti.remove_trailing_nott ())
emit_insn (gen_one_cmplsi2 (operands[0], operands[0]));
DONE;
}
[(set_attr "type" "arith")])
As written this pattern could match after register allocation is
complete and thus we can't create new pseudos (the condition TARGET_SH1
controls that behavior). operands[1] won't necessarily be the T
register in that case.
The split condition fails because we can't create new pseudos, so it's
left as-is. At final assembly time the assertion triggers.
the "&& can_create_pseudo ()" part of the split condition should be
moved into the main condition. I think that's all that's necessary to
fix this problem. It'd probably be best of Oleg went through the
various define_insn_and_split patterns that utilize can_create_pseudo in
their split condition and evaluated them.
I only fixed the most obvious cases in my change from this morning. I
don't typically work on the SH port and for changes which aren't
obviously correct, Oleg is in a better position to evaluate the proper fix.
jeff
