This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch] Change to argument promotion in fixed conversion library calls


On 11/06/2015 08:04 PM, Steve Ellcey  wrote:
When I made this change I had one regression in the GCC testsuite
(gcc.dg/fixed-point/convert-sat.c).  I tracked this down to the
fact that emit_library_call_value_1 does not do any argument promotion
because it does not have the original tree type information for library
calls.  It only knows about modes.  I can't change emit_library_call_value_1
to do the promotion because it does not know whether to do a signed or
unsigned promotion, but expand_fixed_convert could do the conversion
before calling emit_library_call_value_1 and that is what this patch does.

Hmm, difficult. I can see how there would be a problem, but considering how many calls to emit_library_call_* we have, I'm slightly worried whether this is really is a good approach.

On the other hand, there seems to be precedent in this file:

if (GET_MODE_PRECISION (GET_MODE (from)) < GET_MODE_PRECISION (SImode))
        from = convert_to_mode (SImode, from, unsignedp);

The 'real' long term fix for this problem is to have tree types for builtin
functions so the proper promotions can always be done but that is a fairly
large change that I am not willing to tackle right now and it could probably
not be done in time for GCC 6.0 anyway.

Yeah, but I agree that this is the real fix. We should aim to get rid of the emit_library_call functions.

+  if (SCALAR_INT_MODE_P (from_mode))
+    {
+      /*  If we need to promote the integer function argument we need to do

Extra space at the start of the comment.

+	  it here instead of inside emit_library_call_value because here we
+	  know if we should be doing a signed or unsigned promotion.  */
+
+      machine_mode arg_mode;
+      int unsigned_p = 0;
+
+      arg_mode = promote_function_mode (NULL_TREE, from_mode,
+					&unsigned_p, NULL_TREE, 0);
+      if (arg_mode != from_mode)
+	{
+	  from = convert_to_mode (arg_mode, from, uintp);
+	  from_mode = arg_mode;
+	}
+    }

Move this into a separate function (prepare_libcall_arg)? I'll think about it over the weekend and let others chime in if they want, but I think I'll probably end up approving it with that change.


Bernd


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]