Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 02/12/15 05:40, Alex Velenko wrote: On 09/02/15 23:32, Jeff Law wrote: On 02/03/15 20:03, Bin.Cheng wrote: I looked into the test and can confirm the previous compilation is correct. The cover letter of this patch said IRA mis-handled REQ_EQUIV before, but in this case it is REG_EQUAL that is lost. The full dump (without this patch) after IRA is like: Right, but a REG_EQUIV is generated based on the incoming REG_EQUAL notes in the insn stream. Basically update_equiv_regs will scan insn stream and some REG_EQUAL notes will be promoted to REG_EQUIV notes. Hi Jeff, Do I understand you correctly, that REG_EQUAL notes should not be generated in IRA pass, because some of them may get promoted to REG_EQUIV? Or is there any other reason register r110 should not get REG_EQUAL note? Previously, register r110 was getting REG_EQUAL, not REG_EQUIV note. The reason r110 can not get a REG_EQUIV note is because there are multiple insns that set r110 to different values. The equivalency created by a REG_EQUIV note has to be valid at every point within the function. Thus for any pseudo that holds > 1 distinct value within a function there can be no valid REG_EQUIV notes on the assignments to that pseudo. jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 09/02/15 23:32, Jeff Law wrote: On 02/03/15 20:03, Bin.Cheng wrote: I looked into the test and can confirm the previous compilation is correct. The cover letter of this patch said IRA mis-handled REQ_EQUIV before, but in this case it is REG_EQUAL that is lost. The full dump (without this patch) after IRA is like: Right, but a REG_EQUIV is generated based on the incoming REG_EQUAL notes in the insn stream. Basically update_equiv_regs will scan insn stream and some REG_EQUAL notes will be promoted to REG_EQUIV notes. Hi Jeff, Do I understand you correctly, that REG_EQUAL notes should not be generated in IRA pass, because some of them may get promoted to REG_EQUIV? Or is there any other reason register r110 should not get REG_EQUAL note? Previously, register r110 was getting REG_EQUAL, not REG_EQUIV note. Kind regards, Alex The REG_EQUIV is a function-wide equivalence, meaning that one could substitute the REG_EQUIV note for in any uses of the destination register and still have a valid representation of the program. REG_EQUAL's validity is limited to the point after the insn in which it appears and before the next insn. Before r216169 (with REG_EQUAL in insn9), jumps from basic block 6/7/8 -> 9 can be merged because r110 equals to -1 afterwards. But with the patch, the equal information of r110==-1 in basic block 8 is lost. As a result, jump from 8->9 can't be merged and two additional instructions are generated. > I suppose the REG_EQUAL note is correct in insn9? According to GCCint, it only means r110 set by insn9 will be equal to the value at run time at the end of this insn but not necessarily elsewhere in the function. If you previously got a REG_EQUIV note on any of those insns it was wrong and this is the precise kind of situation that the change was trying to fix. R110 can have the value -1 (BB6, BB7, BB8) or 0 (BB5). Thus there is no single value across the entire function that one can validly use for r110. I think you could mark this as a missed optimization, but not a regresssion since the desired output was relying on a bug in the compiler. If I were to start looking at this, my first thought would be to look at why we have multiple sets of r110, particularly if there are lifetimes that are disjoint. I also found another problem (or mis-leading?) with the document: "Thus, compiler passes prior to register allocation need only check for REG_EQUAL notes and passes subsequent to register allocation need only check for REG_EQUIV notes". This seems not true now as in this example, passes after register allocation do take advantage of REG_EQUAL in optimization and we can't achieve that by using REG_EQUIV. I think that's a long standing (and incorrect) generalization. IIRC we can get a REG_EQUIV note earlier for certain argument setup situations. And I think it's been the case for a long time that a pass after reload could try to exploit REG_EQUAL notes. jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 02/10/15 03:51, Ajit Kumar Agarwal wrote: R110 can have the value -1 (BB6, BB7, BB8) or 0 (BB5). Thus there is no single value across the entire function that one can validly use for r110. Does the value of R110 should not change across all the callee path from the given caller functions. If the value is aliased, then how the call side affects should make sure the value remains same across all the callee chain path from the given caller function. I am curious to know how the value remain constant throughout the function is identified in case of aliasing and the interprocedural case. Pseudo registers are allowed to have aliases. Jeff
RE: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
-Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-ow...@gcc.gnu.org] On Behalf Of Jeff Law Sent: Tuesday, February 10, 2015 5:03 AM To: Bin.Cheng Cc: Alex Velenko; Felix Yang; Yangfei (Felix); Marcus Shawcroft; GCC Patches; vmaka...@redhat.com Subject: Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition On 02/03/15 20:03, Bin.Cheng wrote: > I looked into the test and can confirm the previous compilation is correct. > The cover letter of this patch said IRA mis-handled REQ_EQUIV before, > but in this case it is REG_EQUAL that is lost. The full dump (without > this patch) after IRA is like: Right, but a REG_EQUIV is generated based on the incoming REG_EQUAL notes in the insn stream. Basically update_equiv_regs will scan insn stream and some REG_EQUAL notes will be promoted to REG_EQUIV notes. The REG_EQUIV is a function-wide equivalence, meaning that one could substitute the REG_EQUIV note for in any uses of the destination register and still have a valid representation of the program. REG_EQUAL's validity is limited to the point after the insn in which it appears and before the next insn. > > Before r216169 (with REG_EQUAL in insn9), jumps from basic block 6/7/8 > -> 9 can be merged because r110 equals to -1 afterwards. But with the > patch, the equal information of r110==-1 in basic block 8 is lost. As > a result, jump from 8->9 can't be merged and two additional > instructions are generated. > > I suppose the REG_EQUAL note is correct in insn9? According to > GCCint, it only means r110 set by insn9 will be equal to the value at > run time at the end of this insn but not necessarily elsewhere in the > function. >>If you previously got a REG_EQUIV note on any of those insns it was wrong and >>this is the precise kind of situation that the change was trying to fix. >>R110 can have the value -1 (BB6, BB7, BB8) or 0 (BB5). Thus there is no >>single value across the entire function that one can validly use for r110. Does the value of R110 should not change across all the callee path from the given caller functions. If the value is aliased, then how the call side affects should make sure the value remains same across all the callee chain path from the given caller function. I am curious to know how the value remain constant throughout the function is identified in case of aliasing and the interprocedural case. >>I think you could mark this as a missed optimization, but not a regresssion >>since the desired output was relying on a bug in the compiler. >> >>If I were to start looking at this, my first thought would be to look at why >>we have multiple sets of r110, particularly if there are lifetimes that are >>disjoint. > > I also found another problem (or mis-leading?) with the document: > "Thus, compiler passes prior to register allocation need only check > for REG_EQUAL notes and passes subsequent to register allocation need > only check for REG_EQUIV notes". This seems not true now as in this > example, passes after register allocation do take advantage of > REG_EQUAL in optimization and we can't achieve that by using > REG_EQUIV. >>I think that's a long standing (and incorrect) generalization. IIRC we can >>get a REG_EQUIV note earlier for certain argument setup situations. >>And I think it's been the case for a long time that a pass after reload could try to exploit REG_EQUAL notes. Thanks & Regards Ajit jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 02/03/15 20:03, Bin.Cheng wrote: I looked into the test and can confirm the previous compilation is correct. The cover letter of this patch said IRA mis-handled REQ_EQUIV before, but in this case it is REG_EQUAL that is lost. The full dump (without this patch) after IRA is like: Right, but a REG_EQUIV is generated based on the incoming REG_EQUAL notes in the insn stream. Basically update_equiv_regs will scan insn stream and some REG_EQUAL notes will be promoted to REG_EQUIV notes. The REG_EQUIV is a function-wide equivalence, meaning that one could substitute the REG_EQUIV note for in any uses of the destination register and still have a valid representation of the program. REG_EQUAL's validity is limited to the point after the insn in which it appears and before the next insn. Before r216169 (with REG_EQUAL in insn9), jumps from basic block 6/7/8 -> 9 can be merged because r110 equals to -1 afterwards. But with the patch, the equal information of r110==-1 in basic block 8 is lost. As a result, jump from 8->9 can't be merged and two additional instructions are generated. > I suppose the REG_EQUAL note is correct in insn9? According to GCCint, it only means r110 set by insn9 will be equal to the value at run time at the end of this insn but not necessarily elsewhere in the function. If you previously got a REG_EQUIV note on any of those insns it was wrong and this is the precise kind of situation that the change was trying to fix. R110 can have the value -1 (BB6, BB7, BB8) or 0 (BB5). Thus there is no single value across the entire function that one can validly use for r110. I think you could mark this as a missed optimization, but not a regresssion since the desired output was relying on a bug in the compiler. If I were to start looking at this, my first thought would be to look at why we have multiple sets of r110, particularly if there are lifetimes that are disjoint. I also found another problem (or mis-leading?) with the document: "Thus, compiler passes prior to register allocation need only check for REG_EQUAL notes and passes subsequent to register allocation need only check for REG_EQUIV notes". This seems not true now as in this example, passes after register allocation do take advantage of REG_EQUAL in optimization and we can't achieve that by using REG_EQUIV. I think that's a long standing (and incorrect) generalization. IIRC we can get a REG_EQUIV note earlier for certain argument setup situations. And I think it's been the case for a long time that a pass after reload could try to exploit REG_EQUAL notes. jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On Wed, Feb 4, 2015 at 12:28 AM, Jeff Law wrote:
> On 02/03/15 01:29, Bin.Cheng wrote:
>>
>>
>> Hmm, if I understand correctly, it's a code size regression, so I
>> don't think it's appropriate to adapt the test case. Either the patch
>> or something else in GCC is doing wrong, right?
>>
>> Hi Alex, could you please file a PR with full dump information for
>> tracking?
>
> But if the code size regression is due to the older compiler incorrectly
> handling the promotion of REG_EQUAL to REG_EQUIV notes, then the test
> absolutely does need updating as the codesize was dependent on incorrect
> behaviour in the compiler.
Hi Jeff,
I looked into the test and can confirm the previous compilation is correct.
The cover letter of this patch said IRA mis-handled REQ_EQUIV before,
but in this case it is REG_EQUAL that is lost. The full dump (without
this patch) after IRA is like:
10: NOTE_INSN_BASIC_BLOCK 2
2: r116:SI=r0:SI
3: r117:SI=r1:SI
REG_DEAD r1:SI
4: r118:SI=r2:SI
REG_DEAD r2:SI
5: NOTE_INSN_FUNCTION_BEG
12: r2:SI=0x1
13: r1:SI=0
15: r0:SI=call [`lseek'] argc:0
REG_DEAD r2:SI
REG_DEAD r1:SI
REG_CALL_DECL `lseek'
16: r111:SI=r0:SI
REG_DEAD r0:SI
17: r2:SI=0x2
18: r1:SI=0
19: r0:SI=r116:SI
REG_DEAD r116:SI
20: r0:SI=call [`lseek'] argc:0
REG_DEAD r2:SI
REG_DEAD r1:SI
REG_CALL_DECL `lseek'
21: r112:SI=r0:SI
REG_DEAD r0:SI
22: cc:CC=cmp(r111:SI,0x)
23: pc={(cc:CC==0)?L46:pc}
REG_DEAD cc:CC
REG_BR_PROB 159
24: NOTE_INSN_BASIC_BLOCK 3
25: cc:CC=cmp(r112:SI,0x)
26: pc={(cc:CC==0)?L50:pc}
REG_DEAD cc:CC
REG_BR_PROB 159
27: NOTE_INSN_BASIC_BLOCK 4
28: NOTE_INSN_DELETED
29: {cc:CC_NOOV=cmp(r112:SI-r111:SI,0);r114:SI=r112:SI-r111:SI;}
REG_DEAD r112:SI
30: pc={(cc:CC_NOOV==0)?L54:pc}
REG_DEAD cc:CC_NOOV
REG_BR_PROB 400
31: NOTE_INSN_BASIC_BLOCK 5
32: [r117:SI]=r111:SI
REG_DEAD r117:SI
REG_DEAD r111:SI
33: [r118:SI]=r114:SI
REG_DEAD r118:SI
REG_DEAD r114:SI
7: r110:SI=0
REG_EQUAL 0
76: pc=L34
77: barrier
46: L46:
45: NOTE_INSN_BASIC_BLOCK 6
8: r110:SI=r111:SI
REG_DEAD r111:SI
REG_EQUAL 0x
78: pc=L34
79: barrier
50: L50:
49: NOTE_INSN_BASIC_BLOCK 7
6: r110:SI=r112:SI
REG_DEAD r112:SI
REG_EQUAL 0x
80: pc=L34
81: barrier
54: L54:
53: NOTE_INSN_BASIC_BLOCK 8
9: r110:SI=0x
REG_EQUAL 0x
34: L34:
35: NOTE_INSN_BASIC_BLOCK 9
40: r0:SI=r110:SI
REG_DEAD r110:SI
41: use r0:SI
Before r216169 (with REG_EQUAL in insn9), jumps from basic block 6/7/8
-> 9 can be merged because r110 equals to -1 afterwards. But with the
patch, the equal information of r110==-1 in basic block 8 is lost. As
a result, jump from 8->9 can't be merged and two additional
instructions are generated.
I suppose the REG_EQUAL note is correct in insn9? According to
GCCint, it only means r110 set by insn9 will be equal to the value at
run time at the end of this insn but not necessarily elsewhere in the
function.
I also found another problem (or mis-leading?) with the document:
"Thus, compiler passes prior to register allocation need only check
for REG_EQUAL notes and passes subsequent to register allocation need
only check for REG_EQUIV notes". This seems not true now as in this
example, passes after register allocation do take advantage of
REG_EQUAL in optimization and we can't achieve that by using
REG_EQUIV.
Thanks,
bin
>
> jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 02/03/15 01:29, Bin.Cheng wrote: Hmm, if I understand correctly, it's a code size regression, so I don't think it's appropriate to adapt the test case. Either the patch or something else in GCC is doing wrong, right? Hi Alex, could you please file a PR with full dump information for tracking? But if the code size regression is due to the older compiler incorrectly handling the promotion of REG_EQUAL to REG_EQUIV notes, then the test absolutely does need updating as the codesize was dependent on incorrect behaviour in the compiler. jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 03/02/15 08:29, Bin.Cheng wrote:
On Tue, Feb 3, 2015 at 3:24 PM, Jeff Law wrote:
On 02/02/15 08:59, Alex Velenko wrote:
On 11/10/14 13:44, Felix Yang wrote:
Hello Jeff,
I see that you have improved the RTL typesafety issue for ira.c,
so I rebased this patch
on the latest trunk and change to use the new list walking
interface.
Bootstrapped on x86_64-SUSE-Linux and make check regression tested.
OK for trunk?
Hi Felix,
I believe your patch causes a regression for arm-none-eabi.
FAIL: gcc.target/arm/pr43920-2.c object-size text <= 54
FAIL: gcc.target/arm/pr43920-2.c scan-assembler-times pop 2
This happens because your patch stops reuse of code for
" return -1;" statements in pr43920-2.c.
As far as I investigated, your patch prevents adding "(expr_list (-1)
(nil)" in ira pass, which prevents jump2 optimization from happening.
So before, in ira pass I could see:
"(insn 9 53 34 8 (set (reg:SI 110 [ D.4934 ])
(const_int -1 [0x]))
/work/fsf-trunk-ref-2/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:20
613
{*thumb2_movsi_vfp}
(expr_list:REG_EQUAL (const_int -1 [0x])
(nil)))"
But with your patch I get
"(insn 9 53 34 8 (set (reg:SI 110 [ D.5322 ])
(const_int -1 [0x]))
/work/fsf-trunk-2/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:20
615 {*thumb2_movsi_vfp}
(nil))"
This causes a code generation regression and needs to be fixed.
Kind regards,
We'd need to see the full dumps. In particular is reg110 set anywhere else?
If so then the change is doing precisely what it should be doing and the
test needs to be updated to handle the different code we generate.
Hmm, if I understand correctly, it's a code size regression, so I
don't think it's appropriate to adapt the test case. Either the patch
or something else in GCC is doing wrong, right?
Hi Alex, could you please file a PR with full dump information for tracking?
Thanks,
bin
Hi Bin,
Created bugzilla ticket, as requested:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64916
This test already existed in the testsuite, it is not new.
Kind regards,
Alex
Jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On Tue, Feb 3, 2015 at 3:24 PM, Jeff Law wrote:
> On 02/02/15 08:59, Alex Velenko wrote:
>>
>> On 11/10/14 13:44, Felix Yang wrote:
>>>
>>> Hello Jeff,
>>>
>>> I see that you have improved the RTL typesafety issue for ira.c,
>>> so I rebased this patch
>>> on the latest trunk and change to use the new list walking
>>> interface.
>>> Bootstrapped on x86_64-SUSE-Linux and make check regression tested.
>>> OK for trunk?
>>
>> Hi Felix,
>> I believe your patch causes a regression for arm-none-eabi.
>> FAIL: gcc.target/arm/pr43920-2.c object-size text <= 54
>> FAIL: gcc.target/arm/pr43920-2.c scan-assembler-times pop 2
>>
>> This happens because your patch stops reuse of code for
>> " return -1;" statements in pr43920-2.c.
>>
>> As far as I investigated, your patch prevents adding "(expr_list (-1)
>> (nil)" in ira pass, which prevents jump2 optimization from happening.
>>
>> So before, in ira pass I could see:
>> "(insn 9 53 34 8 (set (reg:SI 110 [ D.4934 ])
>> (const_int -1 [0x]))
>> /work/fsf-trunk-ref-2/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:20
>> 613
>> {*thumb2_movsi_vfp}
>> (expr_list:REG_EQUAL (const_int -1 [0x])
>> (nil)))"
>> But with your patch I get
>> "(insn 9 53 34 8 (set (reg:SI 110 [ D.5322 ])
>> (const_int -1 [0x]))
>> /work/fsf-trunk-2/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:20
>> 615 {*thumb2_movsi_vfp}
>> (nil))"
>>
>> This causes a code generation regression and needs to be fixed.
>> Kind regards,
>
> We'd need to see the full dumps. In particular is reg110 set anywhere else?
> If so then the change is doing precisely what it should be doing and the
> test needs to be updated to handle the different code we generate.
Hmm, if I understand correctly, it's a code size regression, so I
don't think it's appropriate to adapt the test case. Either the patch
or something else in GCC is doing wrong, right?
Hi Alex, could you please file a PR with full dump information for tracking?
Thanks,
bin
>
> Jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 02/02/15 08:59, Alex Velenko wrote:
On 11/10/14 13:44, Felix Yang wrote:
Hello Jeff,
I see that you have improved the RTL typesafety issue for ira.c,
so I rebased this patch
on the latest trunk and change to use the new list walking
interface.
Bootstrapped on x86_64-SUSE-Linux and make check regression tested.
OK for trunk?
Hi Felix,
I believe your patch causes a regression for arm-none-eabi.
FAIL: gcc.target/arm/pr43920-2.c object-size text <= 54
FAIL: gcc.target/arm/pr43920-2.c scan-assembler-times pop 2
This happens because your patch stops reuse of code for
" return -1;" statements in pr43920-2.c.
As far as I investigated, your patch prevents adding "(expr_list (-1)
(nil)" in ira pass, which prevents jump2 optimization from happening.
So before, in ira pass I could see:
"(insn 9 53 34 8 (set (reg:SI 110 [ D.4934 ])
(const_int -1 [0x]))
/work/fsf-trunk-ref-2/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:20 613
{*thumb2_movsi_vfp}
(expr_list:REG_EQUAL (const_int -1 [0x])
(nil)))"
But with your patch I get
"(insn 9 53 34 8 (set (reg:SI 110 [ D.5322 ])
(const_int -1 [0x]))
/work/fsf-trunk-2/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:20
615 {*thumb2_movsi_vfp}
(nil))"
This causes a code generation regression and needs to be fixed.
Kind regards,
We'd need to see the full dumps. In particular is reg110 set anywhere
else? If so then the change is doing precisely what it should be doing
and the test needs to be updated to handle the different code we generate.
Jeff
Re: Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 11/10/14 13:44, Felix Yang wrote:
Hello Jeff,
I see that you have improved the RTL typesafety issue for ira.c,
so I rebased this patch
on the latest trunk and change to use the new list walking interface.
Bootstrapped on x86_64-SUSE-Linux and make check regression tested.
OK for trunk?
Hi Felix,
I believe your patch causes a regression for arm-none-eabi.
FAIL: gcc.target/arm/pr43920-2.c object-size text <= 54
FAIL: gcc.target/arm/pr43920-2.c scan-assembler-times pop 2
This happens because your patch stops reuse of code for
" return -1;" statements in pr43920-2.c.
As far as I investigated, your patch prevents adding "(expr_list (-1)
(nil)" in ira pass, which prevents jump2 optimization from happening.
So before, in ira pass I could see:
"(insn 9 53 34 8 (set (reg:SI 110 [ D.4934 ])
(const_int -1 [0x]))
/work/fsf-trunk-ref-2/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:20 613
{*thumb2_movsi_vfp}
(expr_list:REG_EQUAL (const_int -1 [0x])
(nil)))"
But with your patch I get
"(insn 9 53 34 8 (set (reg:SI 110 [ D.5322 ])
(const_int -1 [0x]))
/work/fsf-trunk-2/src/gcc/gcc/testsuite/gcc.target/arm/pr43920-2.c:20
615 {*thumb2_movsi_vfp}
(nil))"
This causes a code generation regression and needs to be fixed.
Kind regards,
Alex
Index: gcc/ChangeLog
===
--- gcc/ChangeLog(revision 216116)
+++ gcc/ChangeLog(working copy)
@@ -1,3 +1,14 @@
+2014-10-11 Felix Yang
+Jeff Law
+
+* ira.c (struct equivalence): Change member "is_arg_equivalence"
and "replace"
+into boolean bitfields; turn member "loop_depth" into a short
integer; add new
+member "no_equiv" and "reserved".
+(no_equiv): Set no_equiv of struct equivalence if register is marked
+as having no known equivalence.
+(update_equiv_regs): Check all definitions for a multiple-set
+register to make sure that the RHS have the same value.
+
2014-10-11 Martin Liska
PR/63376
Index: gcc/ira.c
===
--- gcc/ira.c(revision 216116)
+++ gcc/ira.c(working copy)
@@ -2902,12 +2902,14 @@ struct equivalence
/* Loop depth is used to recognize equivalences which appear
to be present within the same loop (or in an inner loop). */
- int loop_depth;
+ short loop_depth;
/* Nonzero if this had a preexisting REG_EQUIV note. */
- int is_arg_equivalence;
+ unsigned char is_arg_equivalence : 1;
/* Set when an attempt should be made to replace a register
with the associated src_p entry. */
- char replace;
+ unsigned char replace : 1;
+ /* Set if this register has no known equivalence. */
+ unsigned char no_equiv : 1;
};
/* reg_equiv[N] (where N is a pseudo reg number) is the equivalence
@@ -3255,6 +3257,7 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
if (!REG_P (reg))
return;
regno = REGNO (reg);
+ reg_equiv[regno].no_equiv = 1;
list = reg_equiv[regno].init_insns;
if (list && list->insn () == NULL)
return;
@@ -3381,7 +3384,7 @@ update_equiv_regs (void)
/* If this insn contains more (or less) than a single SET,
only mark all destinations as having no known equivalence. */
- if (set == 0)
+ if (set == NULL_RTX)
{
note_stores (PATTERN (insn), no_equiv, NULL);
continue;
@@ -3476,16 +3479,49 @@ update_equiv_regs (void)
if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
note = NULL_RTX;
- if (DF_REG_DEF_COUNT (regno) != 1
- && (! note
+ if (DF_REG_DEF_COUNT (regno) != 1)
+{
+ bool equal_p = true;
+ rtx_insn_list *list;
+
+ /* If we have already processed this pseudo and determined it
+ can not have an equivalence, then honor that decision. */
+ if (reg_equiv[regno].no_equiv)
+continue;
+
+ if (! note
|| rtx_varies_p (XEXP (note, 0), 0)
|| (reg_equiv[regno].replacement
&& ! rtx_equal_p (XEXP (note, 0),
-reg_equiv[regno].replacement
-{
- no_equiv (dest, set, NULL);
- continue;
+reg_equiv[regno].replacement)))
+{
+ no_equiv (dest, set, NULL);
+ continue;
+}
+
+ list = reg_equiv[regno].init_insns;
+ for (; list; list = list->next ())
+{
+ rtx note_tmp;
+ rtx_insn *insn_tmp;
+
+ insn_tmp = list->insn ();
+ note_tmp = find_reg_note (insn_tmp, REG_EQUAL, NULL_RTX);
+ gcc_assert (note_tmp);
+ if (! rtx_equal_p (XEXP (note, 0), XEXP (note_tmp, 0)))
+{
+ equal_p = false;
+ break;
+}
+}
+
+ if (! equal_p)
+{
+ no_equiv (dest, set, NULL);
+ c
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 10/11/14 06:44, Felix Yang wrote: Hello Jeff, I see that you have improved the RTL typesafety issue for ira.c, so I rebased this patch on the latest trunk and change to use the new list walking interface. Bootstrapped on x86_64-SUSE-Linux and make check regression tested. OK for trunk? OK for the trunk. Thanks for your patience. jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
Hello Jeff,
I see that you have improved the RTL typesafety issue for ira.c,
so I rebased this patch
on the latest trunk and change to use the new list walking interface.
Bootstrapped on x86_64-SUSE-Linux and make check regression tested.
OK for trunk?
Index: gcc/ChangeLog
===
--- gcc/ChangeLog(revision 216116)
+++ gcc/ChangeLog(working copy)
@@ -1,3 +1,14 @@
+2014-10-11 Felix Yang
+Jeff Law
+
+* ira.c (struct equivalence): Change member "is_arg_equivalence"
and "replace"
+into boolean bitfields; turn member "loop_depth" into a short
integer; add new
+member "no_equiv" and "reserved".
+(no_equiv): Set no_equiv of struct equivalence if register is marked
+as having no known equivalence.
+(update_equiv_regs): Check all definitions for a multiple-set
+register to make sure that the RHS have the same value.
+
2014-10-11 Martin Liska
PR/63376
Index: gcc/ira.c
===
--- gcc/ira.c(revision 216116)
+++ gcc/ira.c(working copy)
@@ -2902,12 +2902,14 @@ struct equivalence
/* Loop depth is used to recognize equivalences which appear
to be present within the same loop (or in an inner loop). */
- int loop_depth;
+ short loop_depth;
/* Nonzero if this had a preexisting REG_EQUIV note. */
- int is_arg_equivalence;
+ unsigned char is_arg_equivalence : 1;
/* Set when an attempt should be made to replace a register
with the associated src_p entry. */
- char replace;
+ unsigned char replace : 1;
+ /* Set if this register has no known equivalence. */
+ unsigned char no_equiv : 1;
};
/* reg_equiv[N] (where N is a pseudo reg number) is the equivalence
@@ -3255,6 +3257,7 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
if (!REG_P (reg))
return;
regno = REGNO (reg);
+ reg_equiv[regno].no_equiv = 1;
list = reg_equiv[regno].init_insns;
if (list && list->insn () == NULL)
return;
@@ -3381,7 +3384,7 @@ update_equiv_regs (void)
/* If this insn contains more (or less) than a single SET,
only mark all destinations as having no known equivalence. */
- if (set == 0)
+ if (set == NULL_RTX)
{
note_stores (PATTERN (insn), no_equiv, NULL);
continue;
@@ -3476,16 +3479,49 @@ update_equiv_regs (void)
if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
note = NULL_RTX;
- if (DF_REG_DEF_COUNT (regno) != 1
- && (! note
+ if (DF_REG_DEF_COUNT (regno) != 1)
+{
+ bool equal_p = true;
+ rtx_insn_list *list;
+
+ /* If we have already processed this pseudo and determined it
+ can not have an equivalence, then honor that decision. */
+ if (reg_equiv[regno].no_equiv)
+continue;
+
+ if (! note
|| rtx_varies_p (XEXP (note, 0), 0)
|| (reg_equiv[regno].replacement
&& ! rtx_equal_p (XEXP (note, 0),
-reg_equiv[regno].replacement
-{
- no_equiv (dest, set, NULL);
- continue;
+reg_equiv[regno].replacement)))
+{
+ no_equiv (dest, set, NULL);
+ continue;
+}
+
+ list = reg_equiv[regno].init_insns;
+ for (; list; list = list->next ())
+{
+ rtx note_tmp;
+ rtx_insn *insn_tmp;
+
+ insn_tmp = list->insn ();
+ note_tmp = find_reg_note (insn_tmp, REG_EQUAL, NULL_RTX);
+ gcc_assert (note_tmp);
+ if (! rtx_equal_p (XEXP (note, 0), XEXP (note_tmp, 0)))
+{
+ equal_p = false;
+ break;
+}
+}
+
+ if (! equal_p)
+{
+ no_equiv (dest, set, NULL);
+ continue;
+}
}
+
/* Record this insn as initializing this register. */
reg_equiv[regno].init_insns
= gen_rtx_INSN_LIST (VOIDmode, insn, reg_equiv[regno].init_insns);
@@ -3514,10 +3550,9 @@ update_equiv_regs (void)
a register used only in one basic block from a MEM. If so, and the
MEM remains unchanged for the life of the register, add a REG_EQUIV
note. */
-
note = find_reg_note (insn, REG_EQUIV, NULL_RTX);
- if (note == 0 && REG_BASIC_BLOCK (regno) >= NUM_FIXED_BLOCKS
+ if (note == NULL_RTX && REG_BASIC_BLOCK (regno) >= NUM_FIXED_BLOCKS
&& MEM_P (SET_SRC (set))
&& validate_equiv_mem (insn, dest, SET_SRC (set)))
note = set_unique_reg_note (insn, REG_EQUIV, copy_rtx (SET_SRC (set)));
@@ -3547,7 +3582,7 @@ update_equiv_regs (void)
reg_equiv[regno].replacement = x;
reg_equiv[regno].src_p = &SET_SRC (set);
- reg_equiv[regno].loop_depth = loop_depth;
+ reg_equiv[regno].loop_depth = (short) loop_depth;
/* Don't mess with things live dur
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 09/27/14 08:48, Felix Yang wrote:
Thanks for the explaination.
I have changed the loop_depth into a short interger hoping that we can
save some memory :-)
Thanks.
Attached please find the updated patch. Bootstrapped and reg-tested on
x86_64-suse-linux.
Please do a final revew once the assignment is ready.
As for the new list walking interface, I choose the function
"no_equiv" and tried the "checked cast" way.
The bad news is that GCC failed to bootstrap with the following change:
Index: ira.c
===
--- ira.c (revision 215536)
+++ ira.c (working copy)
@@ -3242,12 +3242,12 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
void *data ATTRIBUTE_UNUSED)
{
int regno;
- rtx list;
+ rtx_insn_list *list;
if (!REG_P (reg))
return;
regno = REGNO (reg);
- list = reg_equiv[regno].init_insns;
+ list = as_a (reg_equiv[regno].init_insns);
if (list == const0_rtx)
return;
reg_equiv[regno].init_insns = const0_rtx;
@@ -3258,9 +3258,9 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
return;
ira_reg_equiv[regno].defined_p = false;
ira_reg_equiv[regno].init_insns = NULL;
- for (; list; list = XEXP (list, 1))
+ for (; list; list = list->next ())
{
- rtx insn = XEXP (list, 0);
+ rtx_insn *insn = list->insn ();
remove_note (insn, find_reg_note (insn, REG_EQUIV, NULL_RTX));
}
}
Yea. I'm going to post a patch shortly to go ahead with this
conversion. There's a couple issues that come into play.
First const0_rtx is not an INSN, so we *really* don't want it in the
INSN field of an INSN_LIST. That's probably the ICE you're seeing.
const0_rtx is being used to mark pseudos which we've already determined
can't have a valid equivalence. So we just need a different marker.
That different marker must be embeddable in an INSN_LIST node. The
easiest is just a NULL insn ;-)
The other tests for the const0_rtx marker in ira.c need relatively
trivial updating. And in the end we don't need the checked cast at all ;-)
Jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
Thanks for the explaination.
I have changed the loop_depth into a short interger hoping that we can
save some memory :-)
Attached please find the updated patch. Bootstrapped and reg-tested on
x86_64-suse-linux.
Please do a final revew once the assignment is ready.
As for the new list walking interface, I choose the function
"no_equiv" and tried the "checked cast" way.
The bad news is that GCC failed to bootstrap with the following change:
Index: ira.c
===
--- ira.c (revision 215536)
+++ ira.c (working copy)
@@ -3242,12 +3242,12 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
void *data ATTRIBUTE_UNUSED)
{
int regno;
- rtx list;
+ rtx_insn_list *list;
if (!REG_P (reg))
return;
regno = REGNO (reg);
- list = reg_equiv[regno].init_insns;
+ list = as_a (reg_equiv[regno].init_insns);
if (list == const0_rtx)
return;
reg_equiv[regno].init_insns = const0_rtx;
@@ -3258,9 +3258,9 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
return;
ira_reg_equiv[regno].defined_p = false;
ira_reg_equiv[regno].init_insns = NULL;
- for (; list; list = XEXP (list, 1))
+ for (; list; list = list->next ())
{
- rtx insn = XEXP (list, 0);
+ rtx_insn *insn = list->insn ();
remove_note (insn, find_reg_note (insn, REG_EQUIV, NULL_RTX));
}
}
Error message:
...
checking for suffix of object files... configure: error: in
`/home/yangfei/gcc-devel/build/x86_64-unknown-linux-gnu/libgcc':
configure: error: cannot compute suffix of object files: cannot compile
See `config.log' for more details.
make[2]: *** [configure-stage1-target-libgcc] Error 1
make[2]: Leaving directory `/home/yangfei/gcc-devel/build'
make[1]: *** [stage1-bubble] Error 2
make[1]: Leaving directory `/home/yangfei/gcc-devel/build'
make: *** [all] Error 2
I think the code change is OK. Anything I missed?
Cheers,
Felix
On Sat, Sep 27, 2014 at 5:03 AM, Jeff Law wrote:
> On 09/26/14 07:57, Felix Yang wrote:
>>
>> Hi Jeff,
>>
>> Thanks for the suggestions. I updated the patch accordingly.
>>
>> 1. Both my employer(Huawei) and I have signed the copyright
>> assignments with FSF.
>> These assignments are already sent via post two days ago and
>> hopefully should reach FSF in one week.
>> Maybe it's OK to commit this patch now?
>
> Not really. It needs to be accepted by the FSF before we can include the
> work.
>
>>
>> 2. I am not turning member loop_depth of struct equivalence into
>> short integer as GCC API such as bb_loop_depth
>> returns a loop's depth as a 32-bit interger.
>
> There's already other places that assume loops don't nest that deep. Please
> go ahead and change it. And no need to explicitly mark the unused bits.
> That's just a maintenance nightmare in the long term anyway :-)
>
>
>>
>> 3. I find it's kind of difficult to use the new type and
>> interfaces for list walking the init_insns list for this patch.
>> The type of init_insns list is rtx, not rtl_insn_list *. Seems
>> we need to change a lot in order to use the new interface.
>> Not clear about the reason why it is not adjusted when we are
>> transferring to the new interface.
>> Anyway, I think it's better to have another patch fix that issue.
>> OK?
>
> The right way to go is to add a checked cast when we have some code that is
> using the old interface and other code using the new interface. It's
> actually a pretty easy change.
>
> The checked casts effectively mark the limits of where we've been able to
> push the RTL typesafety work. Long term as we push the typesafety work
> further into the compiler many/most of the checked casts will go away.
>
> Unfortunately, that won't work in this case because other code wants to
> store a (const0_rtx) into the insn list. (const0_rtx) isn't an INSN, so the
> checked cast fails and we get a nice abort/ICE.
>
> Conceptually we just need another marker that is an INSN and we might as
> well just convert the whole file to use the new interface at that point.
>
> Consider the request pulled.
>
> The const0-rtx problem may be why this wasn't converted in the first palce.
> Or it may simply have been a time problem. David's done > 250 patches
> around RTL typesafety, but he also has other work to be doing ;-)
>
>>
>> 4. This bug is only reproduceable with my local customized GCC
>> version. So I don't have a testcase then.
>
> OK.
>
> I'll do a final review when I get notice about the copyright assignment from
> the FSF.
>
> jeff
>
Index: gcc/ChangeLog
===
--- gcc/ChangeLog (revision 215658)
+++ gcc/ChangeLog (working copy)
@@ -1,3 +1,14 @@
+2014-09-26 Felix Yang
+ Jeff Law
+
+ * ira.c (struct equivalence): Change member "is_arg_equivalence" and
"replace"
+ into boolean bitfields; turn member "loop_depth" into a short int
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 09/26/14 07:57, Felix Yang wrote: Hi Jeff, Thanks for the suggestions. I updated the patch accordingly. 1. Both my employer(Huawei) and I have signed the copyright assignments with FSF. These assignments are already sent via post two days ago and hopefully should reach FSF in one week. Maybe it's OK to commit this patch now? Not really. It needs to be accepted by the FSF before we can include the work. 2. I am not turning member loop_depth of struct equivalence into short integer as GCC API such as bb_loop_depth returns a loop's depth as a 32-bit interger. There's already other places that assume loops don't nest that deep. Please go ahead and change it. And no need to explicitly mark the unused bits. That's just a maintenance nightmare in the long term anyway :-) 3. I find it's kind of difficult to use the new type and interfaces for list walking the init_insns list for this patch. The type of init_insns list is rtx, not rtl_insn_list *. Seems we need to change a lot in order to use the new interface. Not clear about the reason why it is not adjusted when we are transferring to the new interface. Anyway, I think it's better to have another patch fix that issue. OK? The right way to go is to add a checked cast when we have some code that is using the old interface and other code using the new interface. It's actually a pretty easy change. The checked casts effectively mark the limits of where we've been able to push the RTL typesafety work. Long term as we push the typesafety work further into the compiler many/most of the checked casts will go away. Unfortunately, that won't work in this case because other code wants to store a (const0_rtx) into the insn list. (const0_rtx) isn't an INSN, so the checked cast fails and we get a nice abort/ICE. Conceptually we just need another marker that is an INSN and we might as well just convert the whole file to use the new interface at that point. Consider the request pulled. The const0-rtx problem may be why this wasn't converted in the first palce. Or it may simply have been a time problem. David's done > 250 patches around RTL typesafety, but he also has other work to be doing ;-) 4. This bug is only reproduceable with my local customized GCC version. So I don't have a testcase then. OK. I'll do a final review when I get notice about the copyright assignment from the FSF. jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
Hi Jeff,
Thanks for the suggestions. I updated the patch accordingly.
1. Both my employer(Huawei) and I have signed the copyright
assignments with FSF.
These assignments are already sent via post two days ago and
hopefully should reach FSF in one week.
Maybe it's OK to commit this patch now?
2. I am not turning member loop_depth of struct equivalence into
short integer as GCC API such as bb_loop_depth
returns a loop's depth as a 32-bit interger.
3. I find it's kind of difficult to use the new type and
interfaces for list walking the init_insns list for this patch.
The type of init_insns list is rtx, not rtl_insn_list *. Seems
we need to change a lot in order to use the new interface.
Not clear about the reason why it is not adjusted when we are
transferring to the new interface.
Anyway, I think it's better to have another patch fix that issue. OK?
4. This bug is only reproduceable with my local customized GCC
version. So I don't have a testcase then.
5. This patch bootstrapped on x86_64-suse-linux and reg-tested,
There are no regressions with this patch.
Regression test summary with or without the patch:
=== gcc Summary ===
# of expected passes107986
# of unexpected failures348
# of unexpected successes33
# of expected failures262
# of unsupported tests2089
/home/yangfei/gcc-devel/gcc-build/gcc/xgcc version 5.0.0 20140924
(experimental) (GCC)
--
=== g++ Summary ===
# of expected passes87415
# of unexpected failures276
# of expected failures266
# of unsupported tests3203
/home/yangfei/gcc-devel/gcc-build/gcc/testsuite/g++/../../xg++
version 5.0.0 20140924 (experimental) (GCC)
--
=== libatomic Summary ===
# of expected passes54
=== libgomp tests ===
Running target unix
=== libgomp Summary ===
# of expected passes693
=== libitm tests ===
Running target unix
=== libitm Summary ===
# of expected passes26
# of expected failures3
# of unsupported tests1
=== libstdc++ tests ===
+++ gcc/ChangeLog(working copy)
@@ -1,3 +1,13 @@
+2014-09-26 Felix Yang
+Jeff Law
+
+* ira.c (struct equivalence): Change member "is_arg_equivalence"
and "replace"
+into boolean bitfields; add new member "no_equiv" and "reserved".
+(no_equiv): Set no_equiv of struct equivalence if register is marked
+as having no known equivalence.
+(update_equiv_regs): Check all definitions for a multiple-set
+register to make sure that the RHS have the same value.
+
2014-09-26 Martin Liska
* cgraph.c (cgraph_node::release_body): New argument keep_arguments
Index: gcc/ira.c
===
--- gcc/ira.c(revision 215640)
+++ gcc/ira.c(working copy)
@@ -2896,10 +2896,13 @@ struct equivalence
to be present within the same loop (or in an inner loop). */
int loop_depth;
/* Nonzero if this had a preexisting REG_EQUIV note. */
- int is_arg_equivalence;
+ unsigned char is_arg_equivalence : 1;
/* Set when an attempt should be made to replace a register
with the associated src_p entry. */
- char replace;
+ unsigned char replace : 1;
+ /* Set if this register has no known equivalence. */
+ unsigned char no_equiv : 1;
+ unsigned char reserved : 5;
};
/* reg_equiv[N] (where N is a pseudo reg number) is the equivalence
@@ -3247,6 +3250,7 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
if (!REG_P (reg))
return;
regno = REGNO (reg);
+ reg_equiv[regno].no_equiv = 1;
list = reg_equiv[regno].init_insns;
if (list == const0_rtx)
return;
@@ -3258,7 +3262,7 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
return;
ira_reg_equiv[regno].defined_p = false;
ira_reg_equiv[regno].init_insns = NULL;
- for (; list; list = XEXP (list, 1))
+ for (; list; list = XEXP (list, 1))
{
rtx insn = XEXP (list, 0);
remove_note (insn, find_reg_note (insn, REG_EQUIV, NULL_RTX));
@@ -3373,7 +3377,7 @@ update_equiv_regs (void)
/* If this insn contains more (or less) than a single SET,
only mark all destinations as having no known equivalence. */
- if (set == 0)
+ if (set == NULL_RTX)
{
note_stores (PATTERN (insn), no_equiv, NULL);
continue;
@@ -3467,16 +3471,48 @@ update_equiv_regs (void)
if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
note = NULL_RTX;
- if (DF_REG_DEF_COUNT (regno) != 1
- && (! note
+ if (DF_REG_DEF_COUNT (regno) != 1)
+{
+ rtx list;
+ bool equal_p = true;
+
+ /* If we have already processed this pseudo and determined it
+ can not have an equivalence, then honor that decision. */
+ if (reg_equiv[regno].no_equiv)
+continue;
+
+ if (! note
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 09/24/14 06:07, Felix Yang wrote: Hi Jeff, Thanks for the comments. I updated the patch adding some enhancements. Bootstrapped on x86_64-suse-linux. Please apply this patch if OK for trunk. Three points: 1. For multiple-set register, it is not qualified to have a equiv note once it is marked by no_equiv. The patch is updated with this consideration. Correct. 2. For the rtx_insn_list new interface, I noticed that the old style XEXP accessor macros is still used in function no_equiv. And I choose to the old style macros with this patch and should come up with another patch to fix this issue, OK? I'd rather any new code going in use the updated interfaces. It's certainly OK to have af followup patch which converts more pre-existing code to the new interfaces. 3. For the conditions that an insn on the init_insns list which did not have a note, I reconsider this and find that this can never happens. So I replaced the check with a gcc assertion. OK. Also, I should have asked this earlier, do you have an assignment on file with the FSF, or does your employer have any kind of blanket assignment on file with the FSF? These changes are large enough to require an assignment. Index: gcc/ira.c === --- gcc/ira.c(revision 215550) +++ gcc/ira.c(working copy) @@ -2900,6 +2900,8 @@ struct equivalence /* Set when an attempt should be made to replace a register with the associated src_p entry. */ char replace; + /* Set if this register has no known equivalence. */ + char no_equiv; }; As a follow-up, can you turn is_arg_equivalence, replace and no_equiv into boolean bitfields and turn loop_depth into a short (to match assumptions elsewhere in GCC). The point is to get better packing of these objects and ultimately use less memory. + + /* Check if it is possible that this multiple-set register has + a known equivalence. */ + if (reg_equiv[regno].no_equiv) +continue; This comment is a bit confusing. Please consider something like /* If we have already processed this pseudo and determined it can not have an equivalence, then honor that decision. */ Do you have a testcase we can add to the regression suite? If at all possible please include one.An execution test would be best, but you could also scan the RTL for bogus REG_EQUIV notes. Please update to use the new type and interfaces for list walking the init_insns list. Finally, you need to verify your patch will bootstrap and not cause any regressions in the testsuite. If you're unsure how to do that, let me know. I think we'll be ready to go once those tasks are complete. jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
PING ?
Cheers,
Felix
On Wed, Sep 24, 2014 at 8:07 PM, Felix Yang wrote:
> Hi Jeff,
>
> Thanks for the comments. I updated the patch adding some enhancements.
> Bootstrapped on x86_64-suse-linux. Please apply this patch if OK for
> trunk.
>
> Three points:
> 1. For multiple-set register, it is not qualified to have a equiv
> note once it is marked by no_equiv. The patch is updated with
>this consideration.
> 2. For the rtx_insn_list new interface, I noticed that the old
> style XEXP accessor macros is still used in function no_equiv.
>And I choose to the old style macros with this patch and should
> come up with another patch to fix this issue, OK?
> 3. For the conditions that an insn on the init_insns list which
> did not have a note, I reconsider this and find that this can
>never happens. So I replaced the check with a gcc assertion.
>
>
> Index: gcc/ChangeLog
> ===
> --- gcc/ChangeLog(revision 215550)
> +++ gcc/ChangeLog(working copy)
> @@ -1,3 +1,11 @@
> +2014-09-24 Felix Yang
> +
> +* ira.c (struct equivalence): Add no_equiv member.
> +(no_equiv): Set no_equiv of struct equivalence if register is marked
> +as having no known equivalence.
> +(update_equiv_regs): Check all definitions for a multiple-set
> +register to make sure that the RHS have the same value.
> +
> 2014-09-24 Jakub Jelinek
>
> PR sanitizer/63316
> Index: gcc/ira.c
> ===
> --- gcc/ira.c(revision 215550)
> +++ gcc/ira.c(working copy)
> @@ -2900,6 +2900,8 @@ struct equivalence
>/* Set when an attempt should be made to replace a register
> with the associated src_p entry. */
>char replace;
> + /* Set if this register has no known equivalence. */
> + char no_equiv;
> };
>
> /* reg_equiv[N] (where N is a pseudo reg number) is the equivalence
> @@ -3247,6 +3249,7 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
>if (!REG_P (reg))
> return;
>regno = REGNO (reg);
> + reg_equiv[regno].no_equiv = 1;
>list = reg_equiv[regno].init_insns;
>if (list == const0_rtx)
> return;
> @@ -3258,7 +3261,7 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
> return;
>ira_reg_equiv[regno].defined_p = false;
>ira_reg_equiv[regno].init_insns = NULL;
> - for (; list; list = XEXP (list, 1))
> + for (; list; list = XEXP (list, 1))
> {
>rtx insn = XEXP (list, 0);
>remove_note (insn, find_reg_note (insn, REG_EQUIV, NULL_RTX));
> @@ -3373,7 +3376,7 @@ update_equiv_regs (void)
>
>/* If this insn contains more (or less) than a single SET,
> only mark all destinations as having no known equivalence. */
> - if (set == 0)
> + if (set == NULL_RTX)
> {
>note_stores (PATTERN (insn), no_equiv, NULL);
>continue;
> @@ -3467,16 +3470,48 @@ update_equiv_regs (void)
>if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
> note = NULL_RTX;
>
> - if (DF_REG_DEF_COUNT (regno) != 1
> - && (! note
> + if (DF_REG_DEF_COUNT (regno) != 1)
> +{
> + rtx list;
> + bool equal_p = true;
> +
> + /* Check if it is possible that this multiple-set register has
> + a known equivalence. */
> + if (reg_equiv[regno].no_equiv)
> +continue;
> +
> + if (! note
>|| rtx_varies_p (XEXP (note, 0), 0)
>|| (reg_equiv[regno].replacement
>&& ! rtx_equal_p (XEXP (note, 0),
> -reg_equiv[regno].replacement
> -{
> - no_equiv (dest, set, NULL);
> - continue;
> +reg_equiv[regno].replacement)))
> +{
> + no_equiv (dest, set, NULL);
> + continue;
> +}
> +
> + list = reg_equiv[regno].init_insns;
> + for (; list; list = XEXP (list, 1))
> +{
> + rtx note_tmp, insn_tmp;
> +
> + insn_tmp = XEXP (list, 0);
> + note_tmp = find_reg_note (insn_tmp, REG_EQUAL, NULL_RTX);
> + gcc_assert (note_tmp);
> + if (! rtx_equal_p (XEXP (note, 0), XEXP (note_tmp, 0)))
> +{
> + equal_p = false;
> + break;
> +}
> +}
> +
> + if (! equal_p)
> +{
> + no_equiv (dest, set, NULL);
> + continue;
> +}
> }
> +
>/* Record this insn as initializing this register. */
>reg_equiv[regno].init_insns
> = gen_rtx_INSN_LIST (VOIDmode, insn, reg_equiv[regno].init_insns);
> @@ -3505,10 +3540,9 @@ update_equiv_regs (void)
> a register used only in one basic block from a MEM. If so, and the
> MEM remains unchanged for the life of the register, add a REG_EQUIV
> note. */
> -
>note = find_reg_note (insn, REG
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
Hi Jeff,
Thanks for the comments. I updated the patch adding some enhancements.
Bootstrapped on x86_64-suse-linux. Please apply this patch if OK for trunk.
Three points:
1. For multiple-set register, it is not qualified to have a equiv
note once it is marked by no_equiv. The patch is updated with
this consideration.
2. For the rtx_insn_list new interface, I noticed that the old
style XEXP accessor macros is still used in function no_equiv.
And I choose to the old style macros with this patch and should
come up with another patch to fix this issue, OK?
3. For the conditions that an insn on the init_insns list which
did not have a note, I reconsider this and find that this can
never happens. So I replaced the check with a gcc assertion.
Index: gcc/ChangeLog
===
--- gcc/ChangeLog(revision 215550)
+++ gcc/ChangeLog(working copy)
@@ -1,3 +1,11 @@
+2014-09-24 Felix Yang
+
+* ira.c (struct equivalence): Add no_equiv member.
+(no_equiv): Set no_equiv of struct equivalence if register is marked
+as having no known equivalence.
+(update_equiv_regs): Check all definitions for a multiple-set
+register to make sure that the RHS have the same value.
+
2014-09-24 Jakub Jelinek
PR sanitizer/63316
Index: gcc/ira.c
===
--- gcc/ira.c(revision 215550)
+++ gcc/ira.c(working copy)
@@ -2900,6 +2900,8 @@ struct equivalence
/* Set when an attempt should be made to replace a register
with the associated src_p entry. */
char replace;
+ /* Set if this register has no known equivalence. */
+ char no_equiv;
};
/* reg_equiv[N] (where N is a pseudo reg number) is the equivalence
@@ -3247,6 +3249,7 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
if (!REG_P (reg))
return;
regno = REGNO (reg);
+ reg_equiv[regno].no_equiv = 1;
list = reg_equiv[regno].init_insns;
if (list == const0_rtx)
return;
@@ -3258,7 +3261,7 @@ no_equiv (rtx reg, const_rtx store ATTRIBUTE_UNUSE
return;
ira_reg_equiv[regno].defined_p = false;
ira_reg_equiv[regno].init_insns = NULL;
- for (; list; list = XEXP (list, 1))
+ for (; list; list = XEXP (list, 1))
{
rtx insn = XEXP (list, 0);
remove_note (insn, find_reg_note (insn, REG_EQUIV, NULL_RTX));
@@ -3373,7 +3376,7 @@ update_equiv_regs (void)
/* If this insn contains more (or less) than a single SET,
only mark all destinations as having no known equivalence. */
- if (set == 0)
+ if (set == NULL_RTX)
{
note_stores (PATTERN (insn), no_equiv, NULL);
continue;
@@ -3467,16 +3470,48 @@ update_equiv_regs (void)
if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
note = NULL_RTX;
- if (DF_REG_DEF_COUNT (regno) != 1
- && (! note
+ if (DF_REG_DEF_COUNT (regno) != 1)
+{
+ rtx list;
+ bool equal_p = true;
+
+ /* Check if it is possible that this multiple-set register has
+ a known equivalence. */
+ if (reg_equiv[regno].no_equiv)
+continue;
+
+ if (! note
|| rtx_varies_p (XEXP (note, 0), 0)
|| (reg_equiv[regno].replacement
&& ! rtx_equal_p (XEXP (note, 0),
-reg_equiv[regno].replacement
-{
- no_equiv (dest, set, NULL);
- continue;
+reg_equiv[regno].replacement)))
+{
+ no_equiv (dest, set, NULL);
+ continue;
+}
+
+ list = reg_equiv[regno].init_insns;
+ for (; list; list = XEXP (list, 1))
+{
+ rtx note_tmp, insn_tmp;
+
+ insn_tmp = XEXP (list, 0);
+ note_tmp = find_reg_note (insn_tmp, REG_EQUAL, NULL_RTX);
+ gcc_assert (note_tmp);
+ if (! rtx_equal_p (XEXP (note, 0), XEXP (note_tmp, 0)))
+{
+ equal_p = false;
+ break;
+}
+}
+
+ if (! equal_p)
+{
+ no_equiv (dest, set, NULL);
+ continue;
+}
}
+
/* Record this insn as initializing this register. */
reg_equiv[regno].init_insns
= gen_rtx_INSN_LIST (VOIDmode, insn, reg_equiv[regno].init_insns);
@@ -3505,10 +3540,9 @@ update_equiv_regs (void)
a register used only in one basic block from a MEM. If so, and the
MEM remains unchanged for the life of the register, add a REG_EQUIV
note. */
-
note = find_reg_note (insn, REG_EQUIV, NULL_RTX);
- if (note == 0 && REG_BASIC_BLOCK (regno) >= NUM_FIXED_BLOCKS
+ if (note == NULL_RTX && REG_BASIC_BLOCK (regno) >= NUM_FIXED_BLOCKS
&& MEM_P (SET_SRC (set))
&& validate_equiv_mem (insn, dest, SET_SRC (set)))
note = set_unique_reg_note (insn, REG_EQUIV, copy_rtx (SET_SRC (set)
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 09/23/14 04:51, Felix Yang wrote:
Hi,
Ignore the previous message.
Attached please find the updated patch.
Bootstrapped on x86_64-suse-linux. Please apply this patch if OK for trunk.
Index: gcc/ChangeLog
===
--- gcc/ChangeLog(revision 215500)
+++ gcc/ChangeLog(working copy)
@@ -1,3 +1,8 @@
+2014-09-23 Felix Yang
+
+* ira.c (update_equiv_regs): Check all definitions for a multiple-set
+register to make sure that the RHS have the same value.
+
2014-09-23 Ilya Enkovich
* cfgcleanup.c (try_optimize_cfg): Do not remove label
Index: gcc/ira.c
===
--- gcc/ira.c(revision 215500)
+++ gcc/ira.c(working copy)
@@ -3467,16 +3467,43 @@ update_equiv_regs (void)
if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
note = NULL_RTX;
- if (DF_REG_DEF_COUNT (regno) != 1
- && (! note
+ if (DF_REG_DEF_COUNT (regno) != 1)
+{
+ rtx list;
This should probably be "rtx_insn_list list".
+ list = reg_equiv[regno].init_insns;
+ for (; list; list = XEXP (list, 1))
Please use the next/insn member functions of the rtx_insn_list rather
than the old style XEXP accessor macros.
+{
+ rtx note_tmp, insn_tmp;
+ insn_tmp = XEXP (list, 0);
+ note_tmp = find_reg_note (insn_tmp, REG_EQUAL, NULL_RTX);
+
+ if (note_tmp == 0
+ || ! rtx_equal_p (XEXP (note, 0), XEXP (note_tmp, 0)))
Under what conditions did you find an insn on this list that did not
have a note? There's a deeper question I'm getting to, but let's start
here.
Rather than use "== 0", if you have an RTX use "== NULL_RTX". That
applies to the note_tmp == 0 test above.
Can you make the edits noted above & answer the question WRT insns on
the reg_equiv[].init_insns list without notes and repost?
Thanks,
Jeff
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
Hi,
Ignore the previous message.
Attached please find the updated patch.
Bootstrapped on x86_64-suse-linux. Please apply this patch if OK for trunk.
Index: gcc/ChangeLog
===
--- gcc/ChangeLog(revision 215500)
+++ gcc/ChangeLog(working copy)
@@ -1,3 +1,8 @@
+2014-09-23 Felix Yang
+
+* ira.c (update_equiv_regs): Check all definitions for a multiple-set
+register to make sure that the RHS have the same value.
+
2014-09-23 Ilya Enkovich
* cfgcleanup.c (try_optimize_cfg): Do not remove label
Index: gcc/ira.c
===
--- gcc/ira.c(revision 215500)
+++ gcc/ira.c(working copy)
@@ -3467,16 +3467,43 @@ update_equiv_regs (void)
if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
note = NULL_RTX;
- if (DF_REG_DEF_COUNT (regno) != 1
- && (! note
+ if (DF_REG_DEF_COUNT (regno) != 1)
+{
+ rtx list;
+ bool equal_p = true;
+
+ if (! note
|| rtx_varies_p (XEXP (note, 0), 0)
|| (reg_equiv[regno].replacement
&& ! rtx_equal_p (XEXP (note, 0),
-reg_equiv[regno].replacement
-{
- no_equiv (dest, set, NULL);
- continue;
+reg_equiv[regno].replacement)))
+{
+ no_equiv (dest, set, NULL);
+ continue;
+}
+
+ list = reg_equiv[regno].init_insns;
+ for (; list; list = XEXP (list, 1))
+{
+ rtx note_tmp, insn_tmp;
+ insn_tmp = XEXP (list, 0);
+ note_tmp = find_reg_note (insn_tmp, REG_EQUAL, NULL_RTX);
+
+ if (note_tmp == 0
+ || ! rtx_equal_p (XEXP (note, 0), XEXP (note_tmp, 0)))
+{
+ equal_p = false;
+ break;
+}
+}
+
+ if (! equal_p)
+{
+ no_equiv (dest, set, NULL);
+ continue;
+}
}
+
/* Record this insn as initializing this register. */
reg_equiv[regno].init_insns
= gen_rtx_INSN_LIST (VOIDmode, insn, reg_equiv[regno].init_insns);
Cheers,
Felix
On Tue, Sep 23, 2014 at 6:45 PM, Felix Yang wrote:
> Hi,
> Attached please fined the updated patch.
>
> Cheers,
> Felix
>
>
> On Tue, Sep 23, 2014 at 10:48 AM, Yangfei (Felix)
> wrote:
>>> On 09/22/14 08:40, Felix Yang wrote:
>>> > Hi,
>>> >
>>> > I find that update_equiv_regs in ira.c sets the wrong EQUIV
>>> > reg-note for pseudo with more than one definiton in certain situation.
>>> > Here is a simplified RTL snippet after this function finishs
>>> > handling:
>>> >
>>> > (insn 77 37 78 2 (set (reg:SI 171)
>>> > (const_int 0 [0])) ticket151.c:33 52 {movsi_internal_dsp}
>>> >(expr_list:REG_EQUAL (const_int 0 [0])
>>> > (nil)))
>>> >
>>> > ..
>>> >
>>> > (insn 79 50 53 2 (set (mem/c:SI (reg/f:SI 136) [2 g_728+0 S4 A64])
>>> > (reg:SI 171)) ticket151.c:33 52 {movsi_internal_dsp}
>>> >(expr_list:REG_DEAD (reg:SI 171)
>>> > (nil)))
>>> > (insn 53 79 54 2 (set (mem/c:SI (reg/f:SI 162) [4 g_163+0 S4 A32])
>>> > (reg:SI 163)) 52 {movsi_internal_dsp}
>>> >(expr_list:REG_DEAD (reg:SI 163)
>>> > (expr_list:REG_DEAD (reg/f:SI 162)
>>> > (nil
>>> > (insn 54 53 14 2 (set (reg:SI 171)
>>> > (mem/u/c:SI (symbol_ref/u:SI ("*.LC8") [flags 0x2]) [4 S4
>>> > A32])) ticket151.c:49 52 {movsi_internal_dsp}
>>> >(expr_list:REG_EQUIV (mem/u/c:SI (symbol_ref/u:SI ("*.LC8")
>>> > [flags 0x2]) [4 S4 A32])
>>> > (expr_list:REG_EQUAL (mem/u/c:SI (symbol_ref/u:SI ("*.LC8")
>>> > [flags 0x2]) [4 S4 A32])
>>> > (nil
>>> >
>>> >
>>> > The REG_EQUIV of insn 54 is not correct as pseudo 171 is defined
>>> > in insn 77 with a differerent value.
>>> > This may causes reload replacing pseudo 171 with mem/u/c:SI
>>> > (symbol_ref/u:SI ("*.LC8"), which is wrong.
>>> > A proposed patch for this issue, please comment:
>>> Is it possible it's the conditional just above this one that is incorrect?
>>>
>>> if (DF_REG_DEF_COUNT (regno) != 1
>>>&& (! note
>>>|| rtx_varies_p (XEXP (note, 0), 0)
>>>|| (reg_equiv[regno].replacement
>>>&& ! rtx_equal_p (XEXP (note, 0),
>>>
>>> reg_equiv[regno].replacement
>>> {
>>>no_equiv (dest, set, NULL);
>>>continue;
>>> }
>>>
>>>
>>> ISTM the only time a multiply-set register can have a valid REG_EQUIV note
>>> is if
>>> each and every assignment to that pseudo has the same RHS.
>>>
>>> Jeff
>>
>>
>> Thanks Jeff. Yes, I agree that this is a better place to consider. I am
>> thinking of a better way to fix this.
>>
>> Vladimir, can you shed light on this and give me some suggestions? Than
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
Hi,
Attached please fined the updated patch.
Cheers,
Felix
On Tue, Sep 23, 2014 at 10:48 AM, Yangfei (Felix) wrote:
>> On 09/22/14 08:40, Felix Yang wrote:
>> > Hi,
>> >
>> > I find that update_equiv_regs in ira.c sets the wrong EQUIV
>> > reg-note for pseudo with more than one definiton in certain situation.
>> > Here is a simplified RTL snippet after this function finishs handling:
>> >
>> > (insn 77 37 78 2 (set (reg:SI 171)
>> > (const_int 0 [0])) ticket151.c:33 52 {movsi_internal_dsp}
>> >(expr_list:REG_EQUAL (const_int 0 [0])
>> > (nil)))
>> >
>> > ..
>> >
>> > (insn 79 50 53 2 (set (mem/c:SI (reg/f:SI 136) [2 g_728+0 S4 A64])
>> > (reg:SI 171)) ticket151.c:33 52 {movsi_internal_dsp}
>> >(expr_list:REG_DEAD (reg:SI 171)
>> > (nil)))
>> > (insn 53 79 54 2 (set (mem/c:SI (reg/f:SI 162) [4 g_163+0 S4 A32])
>> > (reg:SI 163)) 52 {movsi_internal_dsp}
>> >(expr_list:REG_DEAD (reg:SI 163)
>> > (expr_list:REG_DEAD (reg/f:SI 162)
>> > (nil
>> > (insn 54 53 14 2 (set (reg:SI 171)
>> > (mem/u/c:SI (symbol_ref/u:SI ("*.LC8") [flags 0x2]) [4 S4
>> > A32])) ticket151.c:49 52 {movsi_internal_dsp}
>> >(expr_list:REG_EQUIV (mem/u/c:SI (symbol_ref/u:SI ("*.LC8")
>> > [flags 0x2]) [4 S4 A32])
>> > (expr_list:REG_EQUAL (mem/u/c:SI (symbol_ref/u:SI ("*.LC8")
>> > [flags 0x2]) [4 S4 A32])
>> > (nil
>> >
>> >
>> > The REG_EQUIV of insn 54 is not correct as pseudo 171 is defined
>> > in insn 77 with a differerent value.
>> > This may causes reload replacing pseudo 171 with mem/u/c:SI
>> > (symbol_ref/u:SI ("*.LC8"), which is wrong.
>> > A proposed patch for this issue, please comment:
>> Is it possible it's the conditional just above this one that is incorrect?
>>
>> if (DF_REG_DEF_COUNT (regno) != 1
>>&& (! note
>>|| rtx_varies_p (XEXP (note, 0), 0)
>>|| (reg_equiv[regno].replacement
>>&& ! rtx_equal_p (XEXP (note, 0),
>>
>> reg_equiv[regno].replacement
>> {
>>no_equiv (dest, set, NULL);
>>continue;
>> }
>>
>>
>> ISTM the only time a multiply-set register can have a valid REG_EQUIV note
>> is if
>> each and every assignment to that pseudo has the same RHS.
>>
>> Jeff
>
>
> Thanks Jeff. Yes, I agree that this is a better place to consider. I am
> thinking of a better way to fix this.
>
> Vladimir, can you shed light on this and give me some suggestions? Thank you.
>
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
> On 09/22/14 08:40, Felix Yang wrote:
> > Hi,
> >
> > I find that update_equiv_regs in ira.c sets the wrong EQUIV
> > reg-note for pseudo with more than one definiton in certain situation.
> > Here is a simplified RTL snippet after this function finishs handling:
> >
> > (insn 77 37 78 2 (set (reg:SI 171)
> > (const_int 0 [0])) ticket151.c:33 52 {movsi_internal_dsp}
> >(expr_list:REG_EQUAL (const_int 0 [0])
> > (nil)))
> >
> > ..
> >
> > (insn 79 50 53 2 (set (mem/c:SI (reg/f:SI 136) [2 g_728+0 S4 A64])
> > (reg:SI 171)) ticket151.c:33 52 {movsi_internal_dsp}
> >(expr_list:REG_DEAD (reg:SI 171)
> > (nil)))
> > (insn 53 79 54 2 (set (mem/c:SI (reg/f:SI 162) [4 g_163+0 S4 A32])
> > (reg:SI 163)) 52 {movsi_internal_dsp}
> >(expr_list:REG_DEAD (reg:SI 163)
> > (expr_list:REG_DEAD (reg/f:SI 162)
> > (nil
> > (insn 54 53 14 2 (set (reg:SI 171)
> > (mem/u/c:SI (symbol_ref/u:SI ("*.LC8") [flags 0x2]) [4 S4
> > A32])) ticket151.c:49 52 {movsi_internal_dsp}
> >(expr_list:REG_EQUIV (mem/u/c:SI (symbol_ref/u:SI ("*.LC8")
> > [flags 0x2]) [4 S4 A32])
> > (expr_list:REG_EQUAL (mem/u/c:SI (symbol_ref/u:SI ("*.LC8")
> > [flags 0x2]) [4 S4 A32])
> > (nil
> >
> >
> > The REG_EQUIV of insn 54 is not correct as pseudo 171 is defined
> > in insn 77 with a differerent value.
> > This may causes reload replacing pseudo 171 with mem/u/c:SI
> > (symbol_ref/u:SI ("*.LC8"), which is wrong.
> > A proposed patch for this issue, please comment:
> Is it possible it's the conditional just above this one that is incorrect?
>
> if (DF_REG_DEF_COUNT (regno) != 1
>&& (! note
>|| rtx_varies_p (XEXP (note, 0), 0)
>|| (reg_equiv[regno].replacement
>&& ! rtx_equal_p (XEXP (note, 0),
>
> reg_equiv[regno].replacement
> {
>no_equiv (dest, set, NULL);
>continue;
> }
>
>
> ISTM the only time a multiply-set register can have a valid REG_EQUIV note is
> if
> each and every assignment to that pseudo has the same RHS.
>
> Jeff
Thanks Jeff. Yes, I agree that this is a better place to consider. I am
thinking of a better way to fix this.
Vladimir, can you shed light on this and give me some suggestions? Thank you.
Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition
On 09/22/14 08:40, Felix Yang wrote:
Hi,
I find that update_equiv_regs in ira.c sets the wrong EQUIV
reg-note for pseudo with more than one definiton in certain situation.
Here is a simplified RTL snippet after this function finishs handling:
(insn 77 37 78 2 (set (reg:SI 171)
(const_int 0 [0])) ticket151.c:33 52 {movsi_internal_dsp}
(expr_list:REG_EQUAL (const_int 0 [0])
(nil)))
..
(insn 79 50 53 2 (set (mem/c:SI (reg/f:SI 136) [2 g_728+0 S4 A64])
(reg:SI 171)) ticket151.c:33 52 {movsi_internal_dsp}
(expr_list:REG_DEAD (reg:SI 171)
(nil)))
(insn 53 79 54 2 (set (mem/c:SI (reg/f:SI 162) [4 g_163+0 S4 A32])
(reg:SI 163)) 52 {movsi_internal_dsp}
(expr_list:REG_DEAD (reg:SI 163)
(expr_list:REG_DEAD (reg/f:SI 162)
(nil
(insn 54 53 14 2 (set (reg:SI 171)
(mem/u/c:SI (symbol_ref/u:SI ("*.LC8") [flags 0x2]) [4 S4
A32])) ticket151.c:49 52 {movsi_internal_dsp}
(expr_list:REG_EQUIV (mem/u/c:SI (symbol_ref/u:SI ("*.LC8")
[flags 0x2]) [4 S4 A32])
(expr_list:REG_EQUAL (mem/u/c:SI (symbol_ref/u:SI ("*.LC8")
[flags 0x2]) [4 S4 A32])
(nil
The REG_EQUIV of insn 54 is not correct as pseudo 171 is defined
in insn 77 with a differerent value.
This may causes reload replacing pseudo 171 with mem/u/c:SI
(symbol_ref/u:SI ("*.LC8"), which is wrong.
A proposed patch for this issue, please comment:
Is it possible it's the conditional just above this one that is incorrect?
if (DF_REG_DEF_COUNT (regno) != 1
&& (! note
|| rtx_varies_p (XEXP (note, 0), 0)
|| (reg_equiv[regno].replacement
&& ! rtx_equal_p (XEXP (note, 0),
reg_equiv[regno].replacement
{
no_equiv (dest, set, NULL);
continue;
}
ISTM the only time a multiply-set register can have a valid REG_EQUIV
note is if each and every assignment to that pseudo has the same RHS.
Jeff
