diff mbox

RFA: Fix mode checks for possibly-constant predicates

Message ID 87oakx6sqs.fsf@e105548-lin.cambridge.arm.com
State New
Headers show

Commit Message

Richard Sandiford June 3, 2015, 9:33 a.m. UTC 
Richard Sandiford <richard.sandiford@arm.com> writes:
> Richard Henderson <rth@redhat.com> writes:
>> On 05/29/2015 10:23 AM, Richard Sandiford wrote:
>>> +  /* Check whether the predicate accepts const scalar ints (which always
>>> +     have a stored mode of VOIDmode, but logically have a real mode)
>>> +     and whether it matches anything besides const scalar ints.  */
>>> +  bool matches_const_scalar_int_p = false;
>>> +  bool matches_other_p = false;
>>> +  for (int i = 0; i < NUM_RTX_CODE; ++i)
>>> +    if (p->codes[i])
>>> +      switch (i)
>>> +	{
>>> +	CASE_CONST_SCALAR_INT:
>>> +	  matches_const_scalar_int_p = true;
>>> +	  break;
>>> +
>>> +	default:
>>> +	  matches_other_p = true;
>>> +	  break;
>>> +	}
>>> +
>>> +  /* There's no need for a mode check if the predicate only accepts
>>> +     constant integers.  The code checks in the predicate are enough
>>> +     to establish that the mode is VOIDmode.
>>> +
>>> +     Note that the predicate itself should check whether a scalar
>>> +     integer is in range of the given mode.  */
>>> +  if (!matches_other_p && !p->codes[CONST_DOUBLE])
>>> +    return;
>>
>> I think perhaps it would be cleaner to not use CASE_CONST_SCALAR_INT,
>> and then do
>>
>>   switch (i)
>>     {
>>     case CONST_INT:
>>     case CONST_WIDE_INT:
>>       matches_const_scalar_int_p = true;
>>       break;
>>
>>     case CONST_DOUBLE:
>>       if (!TARGET_SUPPORTS_WIDE_INT)
>>         matches_const_scalar_int_p = true;
>>       matches_other_p = true;
>>       break;
>>
>>     default:
>>       matches_other_p = true;
>>       break;
>>     }
>>
>>   if (!matches_other_p)
>>     return;
>>
>> Otherwise ok.
>
> Ah, yeah, that's better.  Here's what I applied after testing on
> x86_64-linux-gnu.

This broke the build for !TARGET_SUPPORTS_WIDE_INT because the default
definition was in the wrong part of defaults.h:

#ifdef GCC_INSN_FLAGS_H
/* Dependent default target macro definitions

   This section of defaults.h defines target macros that depend on generated
   headers.  This is a bit awkward:  We want to put all default definitions
   for target macros in defaults.h, but some of the defaults depend on the
   HAVE_* flags defines of insn-flags.h.  But insn-flags.h is not always
   included by files that do include defaults.h.

   Fortunately, the default macro definitions that depend on the HAVE_*
   macros are also the ones that will only be used inside GCC itself, i.e.
   not in the gen* programs or in target objects like libgcc.

   Obviously, it would be best to keep this section of defaults.h as small
   as possible, by converting the macros defined below to target hooks or
   functions.
*/
...
#endif /* GCC_INSN_FLAGS_H  */

The macro is supposed to be a configuration invariant and so there's
no reason for it depend on generated headers.  The same goes for
SWITCHABLE_TARGET (oops).

Tested by making sure that aarch64-linux-gnu builds again.  Committed
as obvious.

Thanks,
Richard


gcc/
	* defaults.h (SWITCHABLE_TARGET, TARGET_SUPPORTS_WIDE_INT): Move out
	of GCC_INSN_FLAGS_H block.
diff mbox

Patch

Index: gcc/defaults.h
===================================================================
--- gcc/defaults.h	2015-06-03 10:21:20.247590112 +0100
+++ gcc/defaults.h	2015-06-03 10:21:27.015513249 +0100
@@ -1253,6 +1253,18 @@  #define STACK_PUSH_CODE PRE_INC
 # define DEFAULT_FLAG_PIE 0
 #endif
 
+#ifndef SWITCHABLE_TARGET
+#define SWITCHABLE_TARGET 0
+#endif
+
+/* If the target supports integers that are wider than two
+   HOST_WIDE_INTs on the host compiler, then the target should define
+   TARGET_SUPPORTS_WIDE_INT and make the appropriate fixups.
+   Otherwise the compiler really is not robust.  */
+#ifndef TARGET_SUPPORTS_WIDE_INT
+#define TARGET_SUPPORTS_WIDE_INT 0
+#endif
+
 #ifdef GCC_INSN_FLAGS_H
 /* Dependent default target macro definitions
 
@@ -1414,18 +1426,6 @@  #define STACK_CHECK_MAX_VAR_SIZE (STACK_
 #define TARGET_VTABLE_USES_DESCRIPTORS 0
 #endif
 
-#ifndef SWITCHABLE_TARGET
-#define SWITCHABLE_TARGET 0
-#endif
-
-/* If the target supports integers that are wider than two
-   HOST_WIDE_INTs on the host compiler, then the target should define
-   TARGET_SUPPORTS_WIDE_INT and make the appropriate fixups.
-   Otherwise the compiler really is not robust.  */
-#ifndef TARGET_SUPPORTS_WIDE_INT
-#define TARGET_SUPPORTS_WIDE_INT 0
-#endif
-
 #ifndef HAVE_simple_return
 #define HAVE_simple_return 0
 static inline rtx