From: Bob Wilson <bwilson@tensilica.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: [Xtensa] new fix for sub-word constant reloads
Date: Fri, 05 Sep 2008 17:30:00 -0000 [thread overview]
Message-ID: <48C16951.6090200@tensilica.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 2939 bytes --]
I've committed this patch as an alternate fix to my patch from 2008-04-03. The
previous patch did not work with IRA, so I reverted it.
It took a while to reproduce the original problem and figure out what was
happening. Xtensa can only load word-sized values from the constant pool, and
we never generate constant pool entries that are smaller than a word. In this
case (for which I don't have a small testcase), the source has a union type with
an HImode element. The initial value (DImode) for one of the union values is
loaded from the constant pool into a pseudo register, and local_alloc forward
propagates the constant pool MEM into an HImode SUBREG. This SUBREG:HI (MEM:DI)
is accepted by the register_operand predicate, since it is before reload.
Reload then fails because it generates an HImode constant pool load.
The patch changes the Xtensa port to use the TARGET_SECONDARY_RELOAD hook and
uses that hook to check for sub-word reloads from the constant pool. It
provides "reload<mode>_literal" instruction patterns (for QI and HI mode) to do
those reloads with a scratch register. The patch also fixes some predicates
that were not handling SUBREGs around constant pool MEMs, and it removes an old
check for SIGN_EXTEND in the secondary_reload hook that was inherited from the
MIPS port (from which it was removed long ago).
Now the really ironic part of this patch is that I could not find any input to
exercise the new reloads. When reload sees that it needs a scratch register to
reload SUBREG:HI (MEM:DI), it decides to reload the inner MEM separately. So,
besides testing the change as-is, I also built and tested in with the following
change to push_reload():
+#if 0
|| (secondary_reload_class (1, rclass, inmode, in) != NO_REGS
&& (secondary_reload_class (1, rclass, GET_MODE (SUBREG_REG (in)),
SUBREG_REG (in))
== NO_REGS))
+#endif
That causes the outer SUBREG to be reloaded, even though it requires a scratch
register. There are no testsuite regressions, with or without the modified
version of push_reload(), and I verified separately (with the patch backported
to 4.2) that the original problem is fixed.
2008-09-05 Bob Wilson <bob.wilson@acm.org>
* config/xtensa/predicates.md (nonimmed_operand, mem_operand): Use
constantpool_mem_p.
(constantpool_operand): New.
(move_operand): Disallow sub-word modes for the constant pool.
* config/xtensa/xtensa.c (TARGET_SECONDARY_RELOAD): Define.
(xtensa_secondary_reload_class): Replace with....
(xtensa_secondary_reload): this function. Remove SIGN_EXTEND check.
Set icode for sub-word reloads from the constant pool.
* config/xtensa/xtensa.h (SECONDARY_INPUT_RELOAD_CLASS): Delete.
(SECONDARY_OUTPUT_RELOAD_CLASS): Delete.
* config/xtensa/xtensa.md (reload<mode>_literal): New.
* config/xtensa/xtensa-protos.h: Update prototypes.
[-- Attachment #2: gcc-reload-hqi-literals.patch --]
[-- Type: text/x-diff, Size: 5923 bytes --]
Index: config/xtensa/predicates.md
===================================================================
--- config/xtensa/predicates.md (revision 139749)
+++ config/xtensa/predicates.md (working copy)
@@ -1,5 +1,5 @@
;; Predicate definitions for Xtensa.
-;; Copyright (C) 2005, 2006, 2007 Free Software Foundation, Inc.
+;; Copyright (C) 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
;;
;; This file is part of GCC.
;;
@@ -37,13 +37,17 @@
;; Non-immediate operand excluding the constant pool.
(define_predicate "nonimmed_operand"
(ior (and (match_operand 0 "memory_operand")
- (match_test "!constantpool_address_p (XEXP (op, 0))"))
+ (match_test "!constantpool_mem_p (op)"))
(match_operand 0 "register_operand")))
;; Memory operand excluding the constant pool.
(define_predicate "mem_operand"
(and (match_operand 0 "memory_operand")
- (match_test "!constantpool_address_p (XEXP (op, 0))")))
+ (match_test "!constantpool_mem_p (op)")))
+
+;; Memory operand in the constant pool.
+(define_predicate "constantpool_operand"
+ (match_test "constantpool_mem_p (op)"))
(define_predicate "mask_operand"
(ior (and (match_code "const_int")
@@ -131,7 +135,9 @@
(define_predicate "move_operand"
(ior
(ior (match_operand 0 "register_operand")
- (match_operand 0 "memory_operand"))
+ (and (match_operand 0 "memory_operand")
+ (match_test "!constantpool_mem_p (op)
+ || GET_MODE_SIZE (mode) % UNITS_PER_WORD == 0")))
(ior (and (match_code "const_int")
(match_test "GET_MODE_CLASS (mode) == MODE_INT
&& xtensa_simm12b (INTVAL (op))"))
Index: config/xtensa/xtensa.c
===================================================================
--- config/xtensa/xtensa.c (revision 139771)
+++ config/xtensa/xtensa.c (working copy)
@@ -216,6 +216,9 @@
#undef TARGET_EXPAND_BUILTIN
#define TARGET_EXPAND_BUILTIN xtensa_expand_builtin
+#undef TARGET_SECONDARY_RELOAD
+#define TARGET_SECONDARY_RELOAD xtensa_secondary_reload
+
struct gcc_target targetm = TARGET_INITIALIZER;
\f
@@ -2840,22 +2843,23 @@
enum reg_class
-xtensa_secondary_reload_class (enum reg_class rclass,
- enum machine_mode mode ATTRIBUTE_UNUSED,
- rtx x, int isoutput)
+xtensa_secondary_reload (bool in_p, rtx x, enum reg_class rclass,
+ enum machine_mode mode, secondary_reload_info *sri)
{
int regno;
- if (GET_CODE (x) == SIGN_EXTEND)
- x = XEXP (x, 0);
- regno = xt_true_regnum (x);
-
- if (!isoutput)
+ if (in_p && constantpool_mem_p (x))
{
- if (rclass == FP_REGS && constantpool_mem_p (x))
+ if (rclass == FP_REGS)
return RL_REGS;
+
+ if (mode == QImode)
+ sri->icode = CODE_FOR_reloadqi_literal;
+ else if (mode == HImode)
+ sri->icode = CODE_FOR_reloadhi_literal;
}
+ regno = xt_true_regnum (x);
if (ACC_REG_P (regno))
return ((rclass == GR_REGS || rclass == RL_REGS) ? NO_REGS : RL_REGS);
if (rclass == ACC_REG)
Index: config/xtensa/xtensa.h
===================================================================
--- config/xtensa/xtensa.h (revision 139771)
+++ config/xtensa/xtensa.h (working copy)
@@ -508,12 +508,6 @@
#define PREFERRED_OUTPUT_RELOAD_CLASS(X, CLASS) \
xtensa_preferred_reload_class (X, CLASS, 1)
-#define SECONDARY_INPUT_RELOAD_CLASS(CLASS, MODE, X) \
- xtensa_secondary_reload_class (CLASS, MODE, X, 0)
-
-#define SECONDARY_OUTPUT_RELOAD_CLASS(CLASS, MODE, X) \
- xtensa_secondary_reload_class (CLASS, MODE, X, 1)
-
/* Return the maximum number of consecutive registers
needed to represent mode MODE in a register of class CLASS. */
#define CLASS_UNITS(mode, size) \
Index: config/xtensa/xtensa.md
===================================================================
--- config/xtensa/xtensa.md (revision 139750)
+++ config/xtensa/xtensa.md (working copy)
@@ -880,6 +880,31 @@
(set_attr "mode" "QI")
(set_attr "length" "2,2,3,3,3,3,3,3")])
+;; Sub-word reloads from the constant pool.
+
+(define_expand "reload<mode>_literal"
+ [(parallel [(match_operand:HQI 0 "register_operand" "=r")
+ (match_operand:HQI 1 "constantpool_operand" "")
+ (match_operand:SI 2 "register_operand" "=&r")])]
+ ""
+{
+ rtx lit, scratch;
+ unsigned word_off, byte_off;
+
+ gcc_assert (GET_CODE (operands[1]) == SUBREG);
+ lit = SUBREG_REG (operands[1]);
+ scratch = operands[2];
+ word_off = SUBREG_BYTE (operands[1]) & ~(UNITS_PER_WORD - 1);
+ byte_off = SUBREG_BYTE (operands[1]) - word_off;
+
+ lit = adjust_address (lit, SImode, word_off);
+ emit_insn (gen_movsi (scratch, lit));
+ emit_insn (gen_mov<mode> (operands[0],
+ gen_rtx_SUBREG (<MODE>mode, scratch, byte_off)));
+
+ DONE;
+})
+
;; 32-bit floating point moves
(define_expand "movsf"
Index: config/xtensa/xtensa-protos.h
===================================================================
--- config/xtensa/xtensa-protos.h (revision 139749)
+++ config/xtensa/xtensa-protos.h (working copy)
@@ -1,5 +1,6 @@
/* Prototypes of target machine for GNU compiler for Xtensa.
- Copyright 2001, 2002, 2003, 2004, 2005, 2007 Free Software Foundation, Inc.
+ Copyright 2001, 2002, 2003, 2004, 2005, 2007, 2008
+ Free Software Foundation, Inc.
Contributed by Bob Wilson (bwilson@tensilica.com) at Tensilica.
This file is part of GCC.
@@ -66,9 +67,10 @@
extern void xtensa_output_literal (FILE *, rtx, enum machine_mode, int);
extern rtx xtensa_return_addr (int, rtx);
extern enum reg_class xtensa_preferred_reload_class (rtx, enum reg_class, int);
-extern enum reg_class xtensa_secondary_reload_class (enum reg_class,
- enum machine_mode, rtx,
- int);
+struct secondary_reload_info;
+extern enum reg_class xtensa_secondary_reload (bool, rtx, enum reg_class,
+ enum machine_mode,
+ struct secondary_reload_info *);
extern void xtensa_initialize_trampoline (rtx, rtx, rtx);
#endif /* RTX_CODE */
reply other threads:[~2008-09-05 17:16 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=48C16951.6090200@tensilica.com \
--to=bwilson@tensilica.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).