From: Hari Sandanagobalane <hariharans@picochip.com>
To: "gcc@gcc.gnu.org" <gcc@gcc.gnu.org>
Subject: reload question
Date: Thu, 11 Aug 2011 16:49:00 -0000 [thread overview]
Message-ID: <4E44080D.1060303@picochip.com> (raw)
Hello all,
I was making some modifications to picochip port and ran into a problem
with cse within reload and I think it is a bug. Can someone familiar
with reload let me know if it is indeed a bug. The c testcase that
caused the problem was
gcc-4.6.0/gcc/testsuite/./gcc.c-torture/execute/991216-2.c.
After IRA, the relevant section of code was.
(insn 127 32 27 2 (set (reg:HI 7 R7)
(const_int 17767 [0x4567])) test.c:14 15 {movhi}
(nil))
(insn 27 127 128 2 (set (mem:HI (reg/f:SI 39 SP) [0 S2 A32])
(reg:HI 7 R7)) test.c:14 15 {movhi}
(nil))
(insn 128 27 129 2 (set (reg:SI 6 R6)
(symbol_ref:SI ("test") [flags 0x41] <function_decl
0x7f9926963800 test>)) test.c:14 18 {movsi_immediate}
(nil))
(insn 129 128 35 2 (set (reg:SI 36 A0)
(reg:SI 6 R6)) test.c:14 16 {movsi_nonconst}
(nil))
(call_insn 35 129 41 2 (parallel [
(call (mem:QI (reg:SI 36 A0) [0 S1 A8])
(const_int 8 [0x8]))
(clobber (reg:SI 38 LR))
]) test.c:14 22 {call_using_register_32bit}
(nil)
(expr_list:REG_DEP_TRUE (use (reg:HI 5 R5))
(expr_list:REG_DEP_TRUE (use (reg:HI 4 R4))
(expr_list:REG_DEP_TRUE (use (reg:HI 2 R2))
(expr_list:REG_DEP_TRUE (use (reg:HI 1 R1))
(expr_list:REG_DEP_TRUE (use (reg:HI 0 R0))
(nil)))))))
(insn 41 35 40 2 (set (reg:HI 0 R0)
(const_int 4 [0x4])) test.c:15 15 {movhi}
(nil))
(insn 40 41 39 2 (set (reg:HI 5 R5)
(const_int -30293 [0xffffffffffff89ab])) test.c:15 15 {movhi}
(nil))
(insn 39 40 42 2 (set (reg:HI 4 R4)
(const_int -12817 [0xffffffffffffcdef])) test.c:15 15 {movhi}
(nil))
(insn 42 39 43 2 (set (reg:HI 1 R1)
(const_int 2 [0x2])) test.c:15 15 {movhi}
(nil))
(insn 43 42 130 2 (set (reg:HI 2 R2)
(const_int 3 [0x3])) test.c:15 15 {movhi}
(nil))
(insn 130 43 36 2 (set (reg:HI 3 R3)
(const_int 85 [0x55])) test.c:15 15 {movhi}
(nil))
(insn 36 130 44 2 (set (mem:HI (plus:SI (reg/f:SI 39 SP)
(const_int 4 [0x4])) [0 S2 A16])
(reg:HI 3 R3)) test.c:15 15 {movhi}
(nil))
(insn 44 36 131 2 (set (reg:HI 3 R3)
(reg:HI 0 R0)) test.c:15 15 {movhi}
(expr_list:REG_EQUAL (const_int 4 [0x4])
(nil)))
(insn 131 44 37 2 (set (reg:HI 6 R6)
(const_int 291 [0x123])) test.c:15 15 {movhi}
(nil))
(insn 37 131 132 2 (set (mem:HI (plus:SI (reg/f:SI 39 SP)
(const_int 2 [0x2])) [0 S2 A32])
(reg:HI 6 R6)) test.c:15 15 {movhi}
(nil))
(insn 132 37 38 2 (set (reg:HI 7 R7)
(const_int 17767 [0x4567])) test.c:15 15 {movhi}
(nil))
(insn 38 132 133 2 (set (mem:HI (reg/f:SI 39 SP) [0 S2 A32])
(reg:HI 7 R7)) test.c:15 15 {movhi}
(nil))
(insn 133 38 134 2 (set (reg:SI 6 R6)
(symbol_ref:SI ("test") [flags 0x41] <function_decl
0x7f9926963800 test>)) test.c:15 18 {movsi_immediate}
(nil))
All of the above code is within the same basic block. Note that R7 is
being set in the first instruction in this segment (insn 127) to the
value 17767. It is also being set to the same value in instruction 132.
But, the interesting thing to note here is that in insn 128 (SI R6) is
being set to a symbol. SI R6 is R[7:6] combined.
But, after reload, this section of code becomes.
(insn 127 32 27 2 (set (reg:HI 7 R7)
(const_int 17767 [0x4567])) test.c:14 15 {movhi}
(nil))
(insn 27 127 128 2 (set (mem:HI (reg/f:SI 39 SP) [0 S2 A32])
(reg:HI 7 R7)) test.c:14 15 {movhi}
(nil))
(insn 128 27 129 2 (set (reg:SI 6 R6)
(symbol_ref:SI ("test") [flags 0x41] <function_decl
0x7f9926963800 test>)) test.c:14 18 {movsi_immediate}
(nil))
(insn 129 128 35 2 (set (reg:SI 36 A0)
(reg:SI 6 R6)) test.c:14 16 {movsi_nonconst}
(nil))
(call_insn 35 129 41 2 (parallel [
(call (mem:QI (reg:SI 36 A0) [0 S1 A8])
(const_int 8 [0x8]))
(clobber (reg:SI 38 LR))
]) test.c:14 22 {call_using_register_32bit}
(nil)
(expr_list:REG_DEP_TRUE (use (reg:HI 5 R5))
(expr_list:REG_DEP_TRUE (use (reg:HI 4 R4))
(expr_list:REG_DEP_TRUE (use (reg:HI 2 R2))
(expr_list:REG_DEP_TRUE (use (reg:HI 1 R1))
(expr_list:REG_DEP_TRUE (use (reg:HI 0 R0))
(nil)))))))
(insn 41 35 40 2 (set (reg:HI 0 R0)
(const_int 4 [0x4])) test.c:15 15 {movhi}
(nil))
(insn 40 41 39 2 (set (reg:HI 5 R5)
(const_int -30293 [0xffffffffffff89ab])) test.c:15 15 {movhi}
(nil))
(insn 39 40 42 2 (set (reg:HI 4 R4)
(const_int -12817 [0xffffffffffffcdef])) test.c:15 15 {movhi}
(nil))
(insn 42 39 43 2 (set (reg:HI 1 R1)
(const_int 2 [0x2])) test.c:15 15 {movhi}
(nil))
(insn 43 42 130 2 (set (reg:HI 2 R2)
(const_int 3 [0x3])) test.c:15 15 {movhi}
(nil))
(insn 130 43 36 2 (set (reg:HI 3 R3)
(const_int 85 [0x55])) test.c:15 15 {movhi}
(nil))
(insn 36 130 44 2 (set (mem:HI (plus:SI (reg/f:SI 39 SP)
(const_int 4 [0x4])) [0 S2 A16])
(reg:HI 3 R3)) test.c:15 15 {movhi}
(nil))
(insn 44 36 131 2 (set (reg:HI 3 R3)
(reg:HI 0 R0)) test.c:15 15 {movhi}
(expr_list:REG_EQUAL (const_int 4 [0x4])
(nil)))
(insn 131 44 37 2 (set (reg:HI 6 R6)
(const_int 291 [0x123])) test.c:15 15 {movhi}
(nil))
(insn 37 131 132 2 (set (mem:HI (plus:SI (reg/f:SI 39 SP)
(const_int 2 [0x2])) [0 S2 A32])
(reg:HI 6 R6)) test.c:15 15 {movhi}
(nil))
(insn 132 37 38 2 (set (reg:HI 7 R7)
(reg:HI 7 R7)) test.c:15 15 {movhi}
(nil))
(insn 38 132 133 2 (set (mem:HI (reg/f:SI 39 SP) [0 S2 A32])
(reg:HI 7 R7)) test.c:15 15 {movhi}
(nil))
(insn 133 38 134 2 (set (reg:SI 6 R6)
(symbol_ref:SI ("test") [flags 0x41] <function_decl
0x7f9926963800 test>)) test.c:15 18 {movsi_immediate}
(nil))
So, the CSE run in postreload seems to assume that R7 still contains
constant 17767, whereas it actually has been overwritten as part of (SI R6).
I looked at the code in postreload.c file to find the cause. In function
reload_cse_move2add, there is a call to note_stores ~line 2030 which
seems to handle multi-reg modes well. It looks at hard_regno_nregs and
resets the information about each register in that set. But this
function is not always called. There are early exit (continue)
statements within the reload_cse_move2add which do not handle multi-reg
modes at all.
The attached patch fixes the problem for me. I think similar code is
probably also needed in move2add_use_add2_insn. Is that correct?
All my experiments were conducted on 4.6.0 release (but 4.6.1 was no
different)
Many thanks for your help.
Regards
Hari
Patch:
Index: postreload.c
===================================================================
--- postreload.c (revision 171932)
+++ postreload.c (working copy)
@@ -1746,6 +1746,8 @@
rtx src = SET_SRC (pat);
int regno = REGNO (reg);
int min_regno = 0;
+ enum machine_mode mode = GET_MODE (reg);
+ unsigned int nregs = nregs = hard_regno_nregs[regno][mode];
bool speed = optimize_bb_for_speed_p (BLOCK_FOR_INSN (insn));
int i;
bool changed = false;
@@ -1807,11 +1809,21 @@
if (validate_change (insn, &SET_SRC (pat), tem, 0))
changed = true;
}
+
reg_set_luid[regno] = move2add_luid;
reg_base_reg[regno] = -1;
reg_mode[regno] = GET_MODE (reg);
reg_symbol_ref[regno] = sym;
reg_offset[regno] = INTVAL (off);
+ if (nregs != 1)
+ {
+ unsigned int endregno = regno + nregs;
+ for (i = regno; i < endregno; i++)
+ {
+ /* Reset the information about this register. */
+ reg_set_luid[i] = 0;
+ }
+ }
return changed;
}
next reply other threads:[~2011-08-11 16:49 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-11 16:49 Hari Sandanagobalane [this message]
-- strict thread matches above, loose matches on Subject: below --
2010-06-23 17:41 Alex Turjan
2010-06-23 18:30 ` Jeff Law
2010-06-23 21:29 ` Alex Turjan
2010-06-23 21:43 ` Jeff Law
2010-06-24 3:44 ` Joern Rennecke
2007-08-10 20:02 Pat Haugen
2007-08-11 0:18 ` Ian Lance Taylor
2007-08-13 14:55 ` Pat Haugen
2007-08-13 17:42 ` Ian Lance Taylor
2007-08-11 11:45 ` Paolo Bonzini
2005-03-16 10:08 Miles Bader
2005-03-16 14:58 ` Bernd Schmidt
2005-03-18 10:35 ` Miles Bader
2005-03-18 14:04 ` Miles Bader
2005-03-22 18:56 ` Ian Lance Taylor
2005-03-25 14:27 ` tm_gccmail
2005-03-25 15:43 ` Ian Lance Taylor
2005-06-02 5:56 ` Miles Bader
2005-03-25 16:30 ` Alan Lehotsky
2003-03-19 12:51 Reload question Joern Rennecke
2003-03-19 19:39 ` Eric Botcazou
2003-03-18 0:25 Eric Botcazou
2002-07-25 13:57 Richard Kenner
2002-07-26 13:35 ` Mark Mitchell
2002-07-26 15:01 ` Bernd Schmidt
2002-07-26 16:35 ` Mark Mitchell
2002-07-25 13:47 Mark Mitchell
2002-07-25 16:19 ` Bernd Schmidt
2002-01-27 4:34 reload question Chris Lattner
1997-09-29 5:34 Reload question Christian Iseli
1997-09-29 8:41 ` Jeffrey A Law
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=4E44080D.1060303@picochip.com \
--to=hariharans@picochip.com \
--cc=gcc@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).