https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102211
Segher Boessenkool <segher at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |meissner at gcc dot gnu.org,
| |segher at gcc dot gnu.org
--- Comment #6 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Jim Wilson from comment #4)
> Yes, moving SI/DI values to FP regs is OK. However, RISC-V requires that FP
> values in FP registers be stored NaN-boxed. So an SFmode value in a 64-bit
> FP reg has the upper 32-bits of all ones, and the lower 32-bits is the
> value. Thus if accessed as a 64-bit value, you get a NaN.
On Power, a (scalar) SF value is usually stored in DF format. It is the insns
that force the outputs to SP, the inputs in general can be anything.
> The hardware may
> trap if you access a 32-bit value which is not properly NaN-boxed.
We used to have such games as well :-) Thankfully it was largely transparent.
> Using
> qemu to check this may not be good enough, as last time I looked at qemu it
> wasn't handling NaN-boxing correctly, but this was over a year ago, so maybe
> it has been fixed since. I don't know.
QEMU in general optimises for speed, not for correct emulation. If you have
inputs that are invalid it will have more surprising outputs.
> This code sequence is not OK
> foo:
> fmv.d.x fa5,a0
> fmul.s fa0,fa0,fa5
> because we are moving a 64-bit DImode value to an FP reg and then treating
> it as SFmode, which is not OK because the value won't be NaN-boxed and may
> trap at run time.
A situation very similar to the Power problem.
> I would think that TARGET_CAN_CHANGE_MODE_CLASS could help here, but it
> isn't being called inside general_operand when called from fwprop1 where the
> bad substitution happens. Because we have a pseudo-register, and it is only
> called for hard registers.
The documentation says
===
@cindex @code{TARGET_CAN_CHANGE_MODE_CLASS} and subreg semantics
The rules above apply to both pseudo @var{reg}s and hard @var{reg}s.
If the semantics are not correct for particular combinations of
@var{m1}, @var{m2} and hard @var{reg}, the target-specific code
must ensure that those combinations are never used. For example:
@smallexample
TARGET_CAN_CHANGE_MODE_CLASS (@var{m2}, @var{m1}, @var{class})
@end smallexample
must be false for every class @var{class} that includes @var{reg}.
===
so the code does not do what the documentation says?
> I don't see a way to fix this as a backend change with current
> validate_subreg, other than by replacing register_operand with
> riscv_register_operand, and putting the subreg check I need inside
> riscv_register_operand. And likewise for any other affected predicate, like
> move_operand. This will be a big change, though a lot of it will be
> mechanical. As an optimization, we can continue to use register_operand in
> any pattern that can't use FP registers.
The first thing that will have to be done is to restore the status quo, to
make your, my, and all other affected targets work again (there very likely
are more, the problems are all in not-so-very normal cases, not to mention
not all targets are tested so heavily -- which reinforces my argument that
there should have been testsuite cases added that trap this on all targets).
After that, yeah, our backends should be improved. That requires some new
stuff in the middle end as well afaics, there really are reasons Power and
RISC-V both did such terrible thing -- but it certainly should not be done
with a knife at the throat, this is some serious re-engineering, it cannot
be done at a snap of the fingers.
> As a middle end change, I need a new hook in general_operand to reject
> subregs that we can't support on RISC-V.
That may be a good design that is suitable for others as well, it is quite
nicely general. Mike, will that do all we need for the SF subregs as well?
A little problem with this is most of our operands do not inherit from
general_operand. A somewhat bigger problem is: what about predicates, do
those not need changes as well, for good machine code quality?
> Or maybe re-add the check I need to validate_subreg as a hook, so it can be
> conditionally enabled for RISC-V.
Yeah... But it should be re-enabled *by default*, to start with.
> We can allow (subreg:SF (reg:DI)) if it gets allocated to an integer
> register. It is only when it is allocated to an FP register that it can't
> work. I don't know offhand if that can be described. But disallowing the
> subreg always for RISC-V is simpler and also works.
(subreg:SF (reg:DI)) does two things at once: an actual subreg, and a bit_cast.
We should not express both of those with the same rtx code, since we do not
allow subregs of subregs (and that is a good thing!)
