Wrong registers used for C-- mutable let in soft FP mode#9782
Wrong registers used for C-- mutable let in soft FP mode#9782xavierleroy merged 2 commits intoocaml:trunkfrom
Conversation
Software emulation of floating-point arithmetic, as in the ARM EABI port, use pairs of pseudoregisters of type Int to represent values of type Float. This is achieved by the `regs_for` method of class `selector_generic`, which defaults to `Reg.createv` but is overriden for ARM EABI so as to perform the transformation Float -> Int,Int on the fly. The method `bind_let_mut` uses `Reg.createv` to associate pseudoregisters to bound variables. This is incorrect in a soft FP context, as a bound variable of type Float will get a Float register nonetheless. `self#regs_for` must be used instead. This is what this commit does.
gasche
left a comment
gasche
left a comment
There was a problem hiding this comment.
The patch does what is says on the tin. This is good to go if the CI is happy.
I wondered how the mistake could have been avoided and here is a suggestion. I remember when Clet_mut was added, Reg.createv was used to imitate the Reg.createv_like call of bind_let. If I understand correclty, Reg.createv_like (there are several uses in Selectgen) is correct, because it reuses register types from a value that was itself create from regs_for and has already done the translation. Suggestion: instead of having to remember to use either Reg.createv_like r or self#regs_for, could have a method self#regs_like r to consistently call a method in both cases?
#9782) Software emulation of floating-point arithmetic, as in the ARM EABI port, use pairs of pseudoregisters of type Int to represent values of type Float. This is achieved by the `regs_for` method of class `selector_generic`, which defaults to `Reg.createv` but is overriden for ARM EABI so as to perform the transformation Float -> Int,Int on the fly. The method `bind_let_mut` uses `Reg.createv` to associate pseudoregisters to bound variables. This is incorrect in a soft FP context, as a bound variable of type Float will get a Float register nonetheless. `self#regs_for` must be used instead. This is what this commit does. (cherry picked from commit 1e71f75)
|
I took the liberty of cherry-picking this to 4.11 branch, commit 3e74be2. It's a low-risk fix for a bug that was introduced in 4.11. |
|
Cherry-picking to 4.11 duly noted. I will release a last beta with this change, the best-fit allocator fix, and maybe #9765 this week. |
4 participants
Software emulation of floating-point arithmetic, as in the ARM EABI port, use pairs of pseudoregisters of type Int to represent values of type Float.
This is achieved by the
regs_formethod of classselector_generic, which defaults toReg.createvbut is overriden for ARM EABI so as to perform the transformation Float -> Int,Int on the fly.The method
bind_let_mutusesReg.createvto associate pseudoregisters to bound variables. This is incorrect in a soft FPcontext, as a bound variable of type Float will get a Float register nonetheless.
self#regs_formust be used instead. This is what this commit does.