[PATCH 1/2] soft-fp: Fix overwritten issue for division in op-4.h

[Joseph Myers]

On Mon, 15 Oct 2018, Zong Li wrote:

> diff --git a/soft-fp/op-4.h b/soft-fp/op-4.h
> index 01b87d0..a407168 100644
> --- a/soft-fp/op-4.h
> +++ b/soft-fp/op-4.h
> @@ -517,10 +517,15 @@
>  	      R##_f[_FP_DIV_MEAT_4_udiv_i] = -1;			\
>  	      if (!_FP_DIV_MEAT_4_udiv_i)				\
>  		break;							\
> -	      __FP_FRAC_SUB_4 (X##_f[3], X##_f[2], X##_f[1], X##_f[0],	\
> +	      UWtype __FP_FRAC_tmp;					\
> +	      __FP_FRAC_SUB_4 (X##_f[2], X##_f[1], X##_f[0], __FP_FRAC_tmp,	\
>  			       Y##_f[2], Y##_f[1], Y##_f[0], 0,		\
>  			       X##_f[2], X##_f[1], X##_f[0],		\
>  			       _FP_DIV_MEAT_4_udiv_n_f[_FP_DIV_MEAT_4_udiv_i]); \
> +	      X##_f[3] = X##_f[2];					\
> +	      X##_f[2] = X##_f[1];					\
> +	      X##_f[1] = X##_f[0];					\
> +	      X##_f[0] = __FP_FRAC_tmp;					\

This approach, using the X array in a shifted form to store the result 
with a single temporary, is not a clean solution.

First, we should work out what the interface *should* be to all the 
FRAC_ADD and FRAC_SUB macros, regarding permitted overlap between the 
output and the inputs.  This requires examining all those macros and their 
uses to determine what consistent or nearly consistent patterns there are 
in that interface.

Given an appropriate choice of interface, all the definitions that do not 
fit that choice, and all the uses that do not fit that choice, then need 
to be fixed.  If that choice says the overlap above it not OK, a whole 
four-word temporary should be defined using _FP_FRAC_DECL_4 for the 
result, and then copied with _FP_FRAC_COPY_4.  The commit message for any 
proposed change needs to describe the conclusions regarding the interface 
for permitted overlaps, and the analysis done to determine what needs to 
change.

I suspect that the intended interface is that *exact* overlap is permitted 
between the output and the *second* input, but not other overlap.  In 
which case at least the definitions of __FP_FRAC_SUB_3 and __FP_FRAC_SUB_4 
look buggy and need changing (they reference y1 and y2 after r1 and r2 
have been set), as well as the user you're changing above (which should 
use a full temporary).

-- 
Joseph S. Myers
joseph@codesourcery.com