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]

[committed] Revert direct_pool_load_p patch


I've decided to revert:

    http://gcc.gnu.org/ml/gcc-patches/2003-09/msg00841.html

since (despite my efforts to check the c-torture output of each backend ;)
it's been causing problems for the PA:

    http://gcc.gnu.org/ml/gcc-patches/2003-11/msg02267.html

and for powerpc:

    http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13186

Although I think both are latent bugs, and Dave has a nice fix for the
PA one, I don't think my patch really merits the instability it has caused.
I should have known better than to touch reload...

Bootstrapped & regression tested on i686-pc-linux-gnu.  I also tested
a unified powerpc-eabi toolchain to verify that it can now build to
completion.  I also built cc1 for s390-linux-gnu to check that I hadn't
screwed up the reversion there.

--------

FWIW, in the powerpc case, we're generating an input reload of the form
(set (reg:DI FPR) (const_int ...)), which doesn't match the constraints
of the move insn.  As I said in the PR notes:

  I think this is a bug in the rs6000 backend, which seems to have
  a combination of:

     - movdi patterns which provide no constant->FPR alternatives.

     - movdi expanders that do nothing with constant->FPR moves,
       at least not if !TARGET_POWERPC64.

     - a definition of secondary_reload_class which says that constants
       can be moved directly into FPRs:

         /* Constants, memory, and FP registers can go into FP registers.  */
         if ((regno == -1 || FP_REGNO_P (regno))
             && (class == FLOAT_REGS || class == NON_SPECIAL_REGS))
           return NO_REGS;

  I'm not sure what the intended behaviour is, but the attached patch
  seems to get past the initial failure.  Unfortunately, there's not much
  chance of it being right.  I guess this should really be handled in the
  move expanders somehow.

geoffk says that the constant is supposed to be forced into memory, but
the move expanders aren't doing this.  I'm not really familiar enough
with the powerpc port to go changing it myself.

Richard


	PR target/13186

	Revert all of the following patch, except the addition of
	hook_bool_machine_mode_true:

	2003-11-02  Richard Sandiford  <rsandifo@redhat.com>

	* Makefile.in (targhooks.o, reload.o): Update dependencies.
	(GTFILES): Add targhooks.c.
	(gt-targhooks.h): New rule; depend on s-gtype.
	* target.h (direct_pool_load_p): New hook.
	* target-def.h (TARGET_DIRECT_POOL_LOAD_P): New macro.
	(TARGET_INITIALIZER): Include it.
	* targhooks.h (default_direct_pool_load_p): Declare.
	(hook_bool_machine_mode_true): Declare.
	* targhooks.c: Include insn-config.h, recog.h, ggc.h and
	gt-targhooks.h.
	(pool_symbol): New variable.
	(default_direct_pool_load_p): New function.
	(hook_bool_machine_mode_true): New function.
	* reload.c: Include target.h.
	(find_reloads): If an alternative will force a constant into memory,
	count an extra reload if constant pool symbols are not valid
	addresses.  If an alternative uses memory to move values between
	registers, count the move as two reloads rather than one.
	* config/s390/s390.c (TARGET_DIRECT_POOL_LOAD_P): Define.
	* doc/tm.texi (TARGET_DIRECT_POOL_LOAD_P): Document.

Index: Makefile.in
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Makefile.in,v
retrieving revision 1.1204
diff -u -d -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.1204 Makefile.in
--- Makefile.in	2 Dec 2003 03:55:20 -0000	1.1204
+++ Makefile.in	4 Dec 2003 09:03:58 -0000
@@ -1525,7 +1525,7 @@ opts.o : opts.c opts.h options.h toplev.
 	output.h $(DIAGNOSTIC_H) $(TM_P_H) $(INSN_ATTR_H) intl.h
 targhooks.o : targhooks.c targhooks.h $(CONFIG_H) $(SYSTEM_H) \
 	coretypes.h $(TREE_H) $(TM_H) $(RTL_H) $(TM_P_H) function.h \
-	output.h toplev.h insn-config.h $(RECOG_H) $(GGC_H) gt-targhooks.h
+	output.h toplev.h
 
 toplev.o : toplev.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) $(RTL_H) \
    function.h flags.h xcoffout.h input.h $(INSN_ATTR_H) output.h $(DIAGNOSTIC_H) \
@@ -1761,7 +1761,7 @@ ra-rewrite.o : ra-rewrite.c $(CONFIG_H) 
    output.h except.h ra.h reload.h insn-config.h
 reload.o : reload.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) flags.h output.h \
    $(EXPR_H) $(OPTABS_H) reload.h $(RECOG_H) hard-reg-set.h insn-config.h \
-   $(REGS_H) function.h real.h toplev.h $(TM_P_H) $(TARGET_H)
+   $(REGS_H) function.h real.h toplev.h $(TM_P_H)
 reload1.o : reload1.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) real.h flags.h \
    $(EXPR_H) $(OPTABS_H) reload.h $(REGS_H) hard-reg-set.h insn-config.h \
    $(BASIC_BLOCK_H) $(RECOG_H) output.h function.h toplev.h $(TM_P_H) \
@@ -2070,8 +2070,7 @@ GTFILES = $(srcdir)/input.h $(srcdir)/co
   $(srcdir)/profile.c $(srcdir)/ra-build.c $(srcdir)/regclass.c \
   $(srcdir)/reg-stack.c $(srcdir)/cfglayout.c $(srcdir)/langhooks.c \
   $(srcdir)/sdbout.c $(srcdir)/stmt.c $(srcdir)/stor-layout.c \
-  $(srcdir)/stringpool.c $(srcdir)/targhooks.c $(srcdir)/tree.c \
-  $(srcdir)/varasm.c \
+  $(srcdir)/stringpool.c $(srcdir)/tree.c $(srcdir)/varasm.c \
   $(out_file) \
   @all_gtfiles@
 
@@ -2088,7 +2087,7 @@ gt-expr.h gt-sdbout.h gt-optabs.h gt-bit
 gt-dwarf2out.h gt-ra-build.h gt-reg-stack.h gt-dwarf2asm.h \
 gt-dbxout.h gt-c-common.h gt-c-decl.h gt-c-parse.h \
 gt-c-pragma.h gtype-c.h gt-input.h gt-cfglayout.h \
-gt-stringpool.h gt-targhooks.h gt-langhooks.h : s-gtype ; @true
+gt-stringpool.h gt-langhooks.h : s-gtype ; @true
 
 gtyp-gen.h: Makefile
 	echo "/* This file is machine generated.  Do not edit.  */" > tmp-gtyp.h
Index: target-def.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/target-def.h,v
retrieving revision 1.62
diff -u -d -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.62 target-def.h
--- target-def.h	16 Nov 2003 19:10:05 -0000	1.62
+++ target-def.h	4 Dec 2003 09:03:58 -0000
@@ -262,8 +262,6 @@ #define TARGET_ATTRIBUTE_TABLE NULL
 /* In cse.c.  */
 #define TARGET_ADDRESS_COST default_address_cost
 
-#define TARGET_DIRECT_POOL_LOAD_P default_direct_pool_load_p
-
 /* In builtins.c.  */
 #define TARGET_INIT_BUILTINS hook_void_void
 #define TARGET_EXPAND_BUILTIN default_expand_builtin
@@ -381,7 +379,6 @@ #define TARGET_INITIALIZER			\
   TARGET_VECTOR_OPAQUE_P,			\
   TARGET_RTX_COSTS,				\
   TARGET_ADDRESS_COST,				\
-  TARGET_DIRECT_POOL_LOAD_P,			\
   TARGET_DWARF_REGISTER_SPAN,                   \
   TARGET_MACHINE_DEPENDENT_REORG,		\
   TARGET_BUILD_BUILTIN_VA_LIST,			\
Index: target.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/target.h,v
retrieving revision 1.72
diff -u -d -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.72 target.h
--- target.h	1 Dec 2003 13:07:13 -0000	1.72
+++ target.h	4 Dec 2003 09:03:58 -0000
@@ -369,8 +369,6 @@ struct gcc_target
      invalid addresses.  */
   int (* address_cost) (rtx x);
 
-  bool (* direct_pool_load_p) (enum machine_mode);
-
   /* Given a register, this hook should return a parallel of registers
      to represent where to find the register pieces.  Define this hook
      if the register and its mode are represented in Dwarf in
Index: targhooks.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/targhooks.h,v
retrieving revision 2.4
diff -u -d -p -F^\([(a-zA-Z0-9_]\|#define\) -r2.4 targhooks.h
--- targhooks.h	2 Nov 2003 09:34:48 -0000	2.4
+++ targhooks.h	4 Dec 2003 09:03:58 -0000
@@ -32,7 +32,5 @@ extern void default_setup_incoming_varar
 extern bool default_strict_argument_naming (CUMULATIVE_ARGS *);
 extern bool default_pretend_outgoing_varargs_named (CUMULATIVE_ARGS *);
 
-extern bool default_direct_pool_load_p (enum machine_mode);
-
 extern bool hook_bool_CUMULATIVE_ARGS_true (CUMULATIVE_ARGS *);
 extern bool hook_bool_machine_mode_true (enum machine_mode);
Index: targhooks.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/targhooks.c,v
retrieving revision 2.9
diff -u -d -p -F^\([(a-zA-Z0-9_]\|#define\) -r2.9 targhooks.c
--- targhooks.c	2 Nov 2003 09:34:48 -0000	2.9
+++ targhooks.c	4 Dec 2003 09:03:58 -0000
@@ -61,9 +61,6 @@ 02111-1307, USA.  */
 #include "target.h"
 #include "tm_p.h"
 #include "target-def.h"
-#include "insn-config.h"
-#include "recog.h"
-#include "ggc.h"
 
 void
 default_external_libcall (rtx fun ATTRIBUTE_UNUSED)
@@ -199,31 +196,6 @@ default_pretend_outgoing_varargs_named(C
 #endif
 }
 
-/* A SYMBOL_REF for a local symbol.  Used by default_direct_pool_load_p.  */
-
-static GTY(()) rtx pool_symbol;
-
-/* See whether a local symbol is a valid address for MODE.  If so, assume
-   that constant pool symbols are also valid addresses, otherwise assume
-   that they aren't.
-
-   ??? This is only an approximation.  We can't test constant pool
-   symbols directly without forcing something into the constant pool.  */
-
-bool
-default_direct_pool_load_p (enum machine_mode mode)
-{
-  if (pool_symbol == 0)
-    {
-      char label[256];
-
-      ASM_GENERATE_INTERNAL_LABEL (label, "LC", 0);
-      pool_symbol = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (label));
-      SYMBOL_REF_FLAGS (pool_symbol) = SYMBOL_FLAG_LOCAL;
-    }
-  return memory_address_p (mode, pool_symbol);
-}
-
 /* Generic hook that takes a CUMULATIVE_ARGS pointer and returns true.  */
 
 bool
@@ -239,5 +211,3 @@ hook_bool_machine_mode_true (enum machin
 {
   return true;
 }
-
-#include "gt-targhooks.h"
Index: reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload.c,v
retrieving revision 1.226
diff -u -d -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.226 reload.c
--- reload.c	1 Dec 2003 16:17:32 -0000	1.226
+++ reload.c	4 Dec 2003 09:03:59 -0000
@@ -104,7 +104,6 @@ #define REG_OK_STRICT
 #include "output.h"
 #include "function.h"
 #include "toplev.h"
-#include "target.h"
 
 #ifndef REGNO_MODE_OK_FOR_BASE_P
 #define REGNO_MODE_OK_FOR_BASE_P(REGNO, MODE) REGNO_OK_FOR_BASE_P (REGNO)
@@ -3370,11 +3369,6 @@ find_reloads (rtx insn, int replace, int
 		  const_to_mem = 1;
 		  if (this_alternative[i] != (int) NO_REGS)
 		    losers++;
-
-		  /* If constant pool symbols are not valid addresses,
-		     count an extra reload for the address.  */
-		  if (!targetm.direct_pool_load_p (operand_mode[i]))
-		    losers++;
 		}
 
 	      /* If we can't reload this value at all, reject this
@@ -3399,21 +3393,6 @@ find_reloads (rtx insn, int replace, int
 	      else if (modified[i] != RELOAD_WRITE && no_input_reloads
 		       && ! const_to_mem)
 		bad = 1;
-
-#ifdef SECONDARY_MEMORY_NEEDED
-	      /* If this alternative would use memory to move a value
-		 between registers, it would need two reloads, one for
-		 the load and one for the store.  Account for the extra
-		 reload here.  */
-	      if (GET_CODE (operand) == REG
-		  && REGNO (operand) < FIRST_PSEUDO_REGISTER
-		  && (enum reg_class) this_alternative[i] != NO_REGS
-		  && (SECONDARY_MEMORY_NEEDED
-		      ((enum reg_class) this_alternative[i],
-		       REGNO_REG_CLASS (REGNO (operand)),
-		       GET_MODE (operand))))
-		losers++;
-#endif
 
 	      /* We prefer to reload pseudos over reloading other things,
 		 since such reloads may be able to be eliminated later.
Index: config/s390/s390.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390.c,v
retrieving revision 1.124
diff -u -d -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.124 s390.c
--- config/s390/s390.c	30 Nov 2003 15:51:36 -0000	1.124
+++ config/s390/s390.c	4 Dec 2003 09:03:59 -0000
@@ -138,8 +138,6 @@ #define TARGET_SCHED_REORDER2 s390_sched
 #define TARGET_RTX_COSTS s390_rtx_costs
 #undef TARGET_ADDRESS_COST
 #define TARGET_ADDRESS_COST s390_address_cost
-#undef TARGET_DIRECT_POOL_LOAD_P
-#define TARGET_DIRECT_POOL_LOAD_P hook_bool_machine_mode_true
 
 #undef TARGET_MACHINE_DEPENDENT_REORG
 #define TARGET_MACHINE_DEPENDENT_REORG s390_reorg
Index: doc/tm.texi
===================================================================
RCS file: /cvs/gcc/gcc/gcc/doc/tm.texi,v
retrieving revision 1.267
diff -u -d -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.267 tm.texi
--- doc/tm.texi	27 Nov 2003 11:24:42 -0000	1.267
+++ doc/tm.texi	4 Dec 2003 09:03:59 -0000
@@ -5414,19 +5414,6 @@ should probably only be given to address
 registers on machines with lots of registers.
 @end deftypefn
 
-@deftypefn {Target Hook} bool TARGET_DIRECT_POOL_LOAD_P (enum machine_mode @var{m})
-This hook should return true if values of mode @var{m} can usually be loaded
-directly from the constant pool, without using an intermediate register
-to hold the address.
-
-The hook is only a heuristic, it has no bearing on correctness.
-If it returns false, reload will be less likely to force constants
-into memory.
-
-The default definition returns true if an ordinary local symbol is
-a valid address.
-@end deftypefn
-
 @node Scheduling
 @section Adjusting the Instruction Scheduler
 


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