https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79912

--- Comment #16 from mpf at gcc dot gnu.org ---
(In reply to Palmer Dabbelt from comment #15)
> Created attachment 40968 [details]
> glibc file that loops
> 
> The suggested patch causes an infinate loop while building glibc for RISC-V.
> The preprocessed source is attached, but I think the right way to fix this
> might be to actually fix the int-in-FP problem in our backend.  If I
> understand correctly we don't actually need riscv_preferred_reload_class()
> but I can instead remove all the integer->FP modes from the mov{q,h,s,d}
> patterns.

OK, I expect the conditions would need even more fine tuning but...

> 
> With just
> 
> static reg_class_t
> riscv_preferred_reload_class (rtx x ATTRIBUTE_UNUSED, reg_class_t rclass)
> {
>   return rclass;
> }

Yes, or remove the hook entirely of course.  I don't think the
preferred_reload_class is really achieving anything in its current form anyway
as I think it boils down to representing the exact same thing as can be
inferred from the register classes that modes are allowed in anyway. I.e. the
register class will naturally reduce to the right one because some registers
are simply not allowed to hold some modes. I believe, but am not an expert,
that this hook is only really necessary for more complex rules but yours are
relatively simple.

> (which I assume is the same as removing preferred_reload_class()) everything
> builds, but I haven't even bothered running the test suite yet.  Looking at
> the mov patterns I can't actually find any that are illegal.  I'm looking
> for something like
> 
>   [(set (match_operand:QI 0 "nonimmediate_operand" "=f")
>         (match_operand:QI 1 "move_operand"         " r"))]
> 
> which would allow an integer to end up in a FP register, as opposed to

It would if an FPR was allowed to hold a register of mode QI but it is not so
this alternative will always fail. It was just confusing LRA and diagnosing the
true issue was harder because of this being present. Better to remove so it
doesn't mislead future debugging efforts.

>   [(set (match_operand:QI 0 "nonimmediate_operand" "=r")
>         (match_operand:QI 1 "move_operand"         " f"))]
> 

This is equally wrong. It will never succeed just like above but it is
describing a situation where a QImode register is in an FPR just like above
except it is already in an FPR here rather than moving to one. There is no
float-mode involved in this pattern. Did you perhaps just mix up the reference
to an FPR regclass with the use of floats?

> which from my understanding is legal, since we can store floats in GPRs.  I
> can understand how the f-operand versions of movqi and movhi don't make any
> sense, so I'll get rid of those, but I think the all our others are OK?
> 
> What am I missing?

Not much from what I can see, just the mix-up that FPR does not imply float.

Reply via email to