diff mbox

[RFC,alpha] : Disable _FloatN on alpha

Message ID CAFULd4Yu34RTLm1x=N8tbAd1gfGfUm2B8kDMC=cBB46ZhdAZPw@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak Aug. 30, 2016, 9 a.m. UTC
Hello!

The _FloatN patch uncovers a problem on alpha OSF/1 ABI when _Float32
variable arguments are passed to a function. As seen from the source,
32bit _Float32 arguments (SFmode) are *not* promoted to 64bit DFmode
when passing to as variable arguments, and this uncovers following
limitation:

Values, passed in FP registers are pushed to a pretend-args area of
called va-arg function in their 64-bit full word mode, so they are
stored as their bit-exact copy from the reg to the memory using STT
instruction. However, when this memory is later accessed as SFmode
value using LDS instruction, this doesn't load SFmode value (as it was
passed in the FP register), since LDS reorders bits on the way from/to
memory. To make things worse, the value can be moved from pretend-args
area as a DImode value and stored as a SImode subreg of this value.

Float arguments, passed to va-arg function work OK, since default c
promoting rules always promote float variable arguments to double.
This is not the case with _Float32.

To illustrate the problem, consider following source:

--cut here--
volatile _Float32 a = 1.0f32, b = 2.5F32, c = -2.5f32;

_Float32
vafn (_Float32 arg1, ...)
{
  __builtin_va_list ap;
  _Float32 ret;

 __builtin_va_start (ap, arg1);
  ret = arg1 + __builtin_va_arg (ap, _Float32);
 __builtin_va_end (ap);
  return ret;
}

int
main (void)
{
  volatile _Float32 r;
  r = vafn (a, c);
  if (r != -1.5f32)
    __builtin_abort ();
  return 0;
}
--cut here--

Please note the lack of promotion in __builtin_va_arg.

This is compiled to following assembly:

vafn:
        ...
        stt $f17,24($30) <- store DFmode to pretend args area and ...
        ldq $1,24($30) <- ... load it as DImode value
        ...
        stl $1,16($30) <- store SImode subreg to stack and ...
        lds $f10,16($30) <- ... load it as SFmode value
        stq $1,40($30) <- (dead DImode store)
        adds $f16,$f10,$f0 <- use SFmode value in $f10
        ...

main:
        ...
        lda $1,a
        lds $f16,0($1)
        lda $1,c
        lds $f17,0($1)
        jsr $26,vafn
        ...

SFmode values are stored in the FP registers in canonical form, as
subset of DFmode values, with 11-bit exponents restricted to the
corresponding single-precision range, and with the 29 low-order
fraction bits restricted to be all zero. IOW, it is not possible to
load SFmode value from the location where the full 64bit value was
stored from the FP register.

I didn't find a way around this limitation, so I propose to disable
_FloatN on alpha with the attached patch.

Uros.

Comments

Joseph Myers Aug. 30, 2016, 12:09 p.m. UTC | #1
On Tue, 30 Aug 2016, Uros Bizjak wrote:

> I didn't find a way around this limitation, so I propose to disable
> _FloatN on alpha with the attached patch.

If your ABI requires binary32 values to be promoted in variable arguments, 
I'd think you could do that with a new target hook that specifies a type 
to promote to in variable arguments (when such promotion otherwise would 
not occur), affecting code generation for both function calls and va_arg 
(but not for unprototyped function calls, only for variable arguments).  
(TS 18661 allows conversion to the same type to act as a convertFormat 
operation, so it's allowed for such argument passing to convert sNaNs to 
qNaNs, which would be the effect of having such conversions to and from 
binary64 in the call path.)

(Ideally of course such a hook would take effect *after* the front end, 
but various existing such hooks operate in the front end so I don't think 
it's a problem for a new hook to do so as well.)
Uros Bizjak Aug. 30, 2016, 1:25 p.m. UTC | #2
On Tue, Aug 30, 2016 at 2:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Tue, 30 Aug 2016, Uros Bizjak wrote:
>
>> I didn't find a way around this limitation, so I propose to disable
>> _FloatN on alpha with the attached patch.
>
> If your ABI requires binary32 values to be promoted in variable arguments,
> I'd think you could do that with a new target hook that specifies a type
> to promote to in variable arguments (when such promotion otherwise would
> not occur), affecting code generation for both function calls and va_arg
> (but not for unprototyped function calls, only for variable arguments).
> (TS 18661 allows conversion to the same type to act as a convertFormat
> operation, so it's allowed for such argument passing to convert sNaNs to
> qNaNs, which would be the effect of having such conversions to and from
> binary64 in the call path.)
>
> (Ideally of course such a hook would take effect *after* the front end,
> but various existing such hooks operate in the front end so I don't think
> it's a problem for a new hook to do so as well.)

IIUC, trying to solve this issue in the front-end would bring us to
float->double promotion rules for variable arguments. The compiler
doesn't convert va-args automatically, it only warns for undefined
situation, e.g.:

--cut here--
#include <stdarg.h>

float sum (float a, ...)
{
  va_list arg;
  float acc = a;

  va_start (arg, a);
  acc += va_arg (arg, float);
  va_end (arg);

  return acc;
}
--cut here--

$ gcc -O2 va.c
va.c: In function ‘sum’:
va.c:9: warning: ‘float’ is promoted to ‘double’ when passed through ‘...’
va.c:9: warning: (so you should pass ‘double’ not ‘float’ to ‘va_arg’)
va.c:9: note: if this code is reached, the program will abort

In a similar way, if we require that _Float32 gets promoted to
_Float64 in va-arg, all the compiler can do is to generate a
target-dependant warning, so user can fix the program. I don't think
this is desirable, as we would have something like:

#ifdef __alpha__
  acc += va_arg (arg, _Float64);
#else
  acc += va_arg (arg, _Float32);
#endif

Maybe better way forward would be to introduce a backend hook to
generate target-dependant load of the argument from the save area.
This way, it would be possible to emit some kind of builtin that
expands to some RTX sequence.

Uros.
Joseph Myers Aug. 30, 2016, 2:04 p.m. UTC | #3
On Tue, 30 Aug 2016, Uros Bizjak wrote:

> IIUC, trying to solve this issue in the front-end would bring us to
> float->double promotion rules for variable arguments. The compiler
> doesn't convert va-args automatically, it only warns for undefined
> situation, e.g.:

No, those would be unchanged.  The hook would mean two things:

* When generating the call, a promotion is inserted for _Float32 like it 
would be for float (if that's the appropriate ABI for this case on this 
architecture).

* When expanding va_arg (ap, _Float32) - which is perfectly valid, unlike 
va_arg (ap, float) - the front end changes the call to look like 
(_Float32) va_arg (ap, _Float64).

> Maybe better way forward would be to introduce a backend hook to
> generate target-dependant load of the argument from the save area.
> This way, it would be possible to emit some kind of builtin that
> expands to some RTX sequence.

Sure, if you can make the back end do the right thing so that plain 
__builtin_va_arg (ap, _Float32) works without the front end needing to 
change the type and insert a promotion at the call site and a conversion 
back to _Float32 at the va_arg site, that's even better.
diff mbox

Patch

diff --git a/gcc/config/alpha/alpha.c b/gcc/config/alpha/alpha.c
index 702cd27..1bf27ca 100644
--- a/gcc/config/alpha/alpha.c
+++ b/gcc/config/alpha/alpha.c
@@ -736,6 +736,20 @@  alpha_vector_mode_supported_p (machine_mode mode)
   return mode == V8QImode || mode == V4HImode || mode == V2SImode;
 }
 
+/* Target hook for floatn_mode.  */
+
+static machine_mode
+alpha_floatn_mode (int n ATTRIBUTE_UNUSED, bool extended ATTRIBUTE_UNUSED)
+{
+  /* Alpha can't pass _Float32 variable arguments.  The function that is
+     receiving a variable number of arguments pushes floating-point values
+     in the remaining incoming registers to the stack as DFmode 64-bit values.
+     These stores are not compatible with SFmode loads due to implicit
+     reordering of bits during SFmode load.  Disable all _Floatn types.  */
+
+  return VOIDmode;
+}
+
 /* Return 1 if this function can directly return via $26.  */
 
 int
@@ -9978,8 +9992,6 @@  alpha_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
 
 #undef TARGET_PROMOTE_FUNCTION_MODE
 #define TARGET_PROMOTE_FUNCTION_MODE default_promote_function_mode_always_promote
-#undef TARGET_PROMOTE_PROTOTYPES
-#define TARGET_PROMOTE_PROTOTYPES hook_bool_const_tree_false
 
 #undef TARGET_FUNCTION_VALUE
 #define TARGET_FUNCTION_VALUE alpha_function_value
@@ -10020,6 +10032,8 @@  alpha_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
 #define TARGET_SCALAR_MODE_SUPPORTED_P alpha_scalar_mode_supported_p
 #undef TARGET_VECTOR_MODE_SUPPORTED_P
 #define TARGET_VECTOR_MODE_SUPPORTED_P alpha_vector_mode_supported_p
+#undef TARGET_FLOATN_MODE
+#define TARGET_FLOATN_MODE alpha_floatn_mode
 
 #undef TARGET_BUILD_BUILTIN_VA_LIST
 #define TARGET_BUILD_BUILTIN_VA_LIST alpha_build_builtin_va_list