Hi Paul,
On 8/30/21 4:16 PM, Paul A. Clarke wrote:
On Fri, Aug 27, 2021 at 08:44:43AM -0500, Bill Schmidt via Gcc-patches wrote:
On 8/23/21 2:03 PM, Paul A. Clarke wrote:
+ __fpscr_save.__fr = __builtin_mffsl ();
As pointed out in the v1 review, __builtin_mffsl is enabled (or supposed to
be) only for POWER9 and later. This will fail to work on POWER8 and earlier
when the new builtins support is complete and this is enforced more
carefully. Please #ifdef and use __builtin_mffs on earlier processors.
Please do this everywhere this occurs.
I think you got some contradictory guidance on this, but trust me, this will
break.
The confusing thing is that _builtin_mffsl is explicitly supported on earlier
processors, if I read the code right (from gcc/config/rs6000/rs6000.md):
--
(define_expand "rs6000_mffsl"
[(set (match_operand:DF 0 "gpc_reg_operand")
(unspec_volatile:DF [(const_int 0)] UNSPECV_MFFSL))]
"TARGET_HARD_FLOAT"
{
/* If the low latency mffsl instruction (ISA 3.0) is available use it,
otherwise fall back to the older mffs instruction to emulate the mffsl
instruction. */
if (!TARGET_P9_MISC)
{
rtx tmp1 = gen_reg_rtx (DFmode);
/* The mffs instruction reads the entire FPSCR. Emulate the mffsl
instruction using the mffs instruction and masking the result. */
emit_insn (gen_rs6000_mffs (tmp1));
...
--
Is that going away? If so, that would be a possible (undesirable?)
API change, no?
Hm, I see. I missed that in the builtins conversion. Apparently
there's nothing in the test suite that verifies this work on P9, which
is a hole that could use fixing. This usage isn't documented anywhere
near the builtin machinery, either.
I'll patch the new builtins code to move this to a more permissive
stanza and document why. You can leave your code as is.
Thanks!
Bill
PC
