This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: revised patch committed.
Thanks Kenny.
My target does not support zero_extract with non-constant values in
the 2nd and 3rd field, so I can't really check this on my existing
code-base and regressions.
Anyway I suspect there is some problem with dse pass, after applying
that patch to df_scan.
It seems that zero_extract is not handled well and as a result dse
ignores the effect of stores with zero_extract on the lhs, and creates
wrong rtl.
Here is an example:
typedef struct _S
{
short a: 14;
short b: 1;
short c: 1;
} S;
short g_short;
void f()
{
short i=0;
S *s = (S*) &i;
s->a = 1;
g_short = *(short*)s;
}
Here is dse pass dump:
;; Function f (f)
starting the processing of deferred insns
ending the processing of deferred insns
df_analyze called
**scanning insn=5
mem: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
after cselib_expand address: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
after canon_rtx address: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
gid=0 offset=4
processing const base store gid=0[4..6)
mems_found = 1, cannot_delete = false
**scanning insn=6
mems_found = 0, cannot_delete = true
**scanning insn=7
mem: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
after cselib_expand address: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
after canon_rtx address: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
gid=0 offset=4
processing const load gid=0[4..6)
trying to replace HImode load in insn 7 from HImode store in insn 5
deferring rescan insn with uid = 7.
deferring rescan insn with uid = 15.
-- replaced the loaded MEM with (reg 43)
mem: (symbol_ref:HI ("g_short") <var_decl 0x2ee40b0 g_short>)
after cselib_expand address: (symbol_ref:HI ("g_short") <var_decl
0x2ee40b0 g_short>)
after canon_rtx address: (symbol_ref:HI ("g_short") <var_decl
0x2ee40b0 g_short>)
gid=1 offset=0
processing const base store gid=1[0..2)
mems_found = 1, cannot_delete = false
Locally deleting insn 5
deferring deletion of insn with uid = 5.
group 0 is frame related group 0(0+2): n p 4, 5
group 1(0+0): n p
starting the processing of deferred insns
deleting insn with uid = 5.
rescanning insn with uid = 7.
deleting insn with uid = 7.
rescanning insn with uid = 15.
deleting insn with uid = 15.
ending the processing of deferred insns
df_analyze called
df_worklist_dataflow_doublequeue:n_basic_blocks 3 n_edges 2 count 3 ( 1)
f
Dataflow summary:
def_info->table_size = 0, use_info->table_size = 8
;; invalidated by call 32 [blink] 33 [cc] 37 [gccReturnValue]
;; hardware regs used 34 [sp] 35 [fp] 36 [arg]
;; regular block artificial uses 34 [sp] 35 [fp] 36 [arg]
;; eh block artificial uses 34 [sp] 35 [fp] 36 [arg]
;; entry block defs 34 [sp] 35 [fp] 36 [arg]
;; exit block uses 34 [sp] 35 [fp]
;; regs ever live
;; ref usage r34={1d,2u} r35={1d,3u} r36={1d,1u} r43={1d,1u}
;; total ref usage 11{4d,7u,0e} in 3{3 regular + 0 call} insns.
( )->[0]->( 2 )
;; bb 0 artificial_defs: { d0(34){ }d1(35){ }d2(36){ }}
;; bb 0 artificial_uses: { }
;; lr in
;; lr use
;; lr def 34 [sp] 35 [fp] 36 [arg]
;; live in
;; live gen 34 [sp] 35 [fp] 36 [arg]
;; live kill
;; lr out 34 [sp] 35 [fp] 36 [arg]
;; live out 34 [sp] 35 [fp] 36 [arg]
( 0 )->[2]->( 1 )
;; bb 2 artificial_defs: { }
;; bb 2 artificial_uses: { u0(34){ }u1(35){ }u2(36){ }}
;; lr in 34 [sp] 35 [fp] 36 [arg]
;; lr use 34 [sp] 35 [fp] 36 [arg]
;; lr def 43
;; live in 34 [sp] 35 [fp] 36 [arg]
;; live gen
;; live kill
;; lr out 34 [sp] 35 [fp] 36 [arg]
;; live out 34 [sp] 35 [fp] 36 [arg]
( 2 )->[1]->( )
;; bb 1 artificial_defs: { }
;; bb 1 artificial_uses: { u6(34){ }u7(35){ }}
;; lr in 34 [sp] 35 [fp]
;; lr use 34 [sp] 35 [fp]
;; lr def
;; live in 34 [sp] 35 [fp]
;; live gen
;; live kill
;; lr out
;; live out
Finding needed instructions:
Adding insn 7 to worklist
Adding insn 6 to worklist
Finished finding needed instructions:
processing block 2 lr out = 34 [sp] 35 [fp] 36 [arg]
Adding insn 15 to worklist
df_worklist_dataflow_doublequeue:n_basic_blocks 3 n_edges 2 count 4 ( 1.3)
doing global processing
df_worklist_dataflow_doublequeue:n_basic_blocks 3 n_edges 2 count 5 ( 1.7)
*** Global dataflow info after analysis.
( )->[0]->( 2 )
in:
gen:
kill: *MISSING*
out: 1, 2
( 0 )->[2]->( 1 )
in: 1, 2
gen:
kill:
out: 1, 2
( 2 )->[1]->( )
in: 1, 2
gen: 1, 2
kill: *MISSING*
out: *MISSING*
starting to process insn 7
v: 1, 2
i = 0, index = 0
failing at i = 0
starting to process insn 6
v: 1, 2
dse: local deletions = 1, global deletions = 0, spill deletions = 0
(note 3 0 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(note 2 3 15 2 NOTE_INSN_FUNCTION_BEG)
(insn 15 2 6 2 c:\mingw\home\amirg\test\test.c:12 (set (reg:HI 43)
(const_int 0 [0x0])) -1 (nil))
(insn 6 15 7 2 c:\mingw\home\amirg\test\test.c:14 (set
(zero_extract:SI (mem/s/j:QI (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4])) [0 S1 A8])
(const_int 14 [0xe])
(const_int 0 [0x0]))
(const_int 1 [0x1])) 29 {*movb_lh} (nil))
(insn 7 6 0 2 c:\mingw\home\amirg\test\test.c:15 (set (mem/c/i:HI
(symbol_ref:HI ("g_short") <var_decl 0x2ee40b0 g_short>) [0 g_short+0
S2 A16])
(reg:HI 43)) 18 {*movhi} (expr_list:REG_DEAD (reg:HI 43)
(nil)))
starting the processing of deferred insns
ending the processing of deferred insns
Note that in insn 7, (mem (plus (reg fp) (const_int 4)) was replaced
by (reg 43), regardless of the fact that the same mem is updated (in
zero_extract) on insn 6.
I'm not sure how to actually fix the problem, but I was able to work
around it by treating zero_extract as wild store in record_store in
dse.c:
--- dse.c
+++ dse.c
@@ -1301,6 +1301,16 @@
return 0;
mem = SET_DEST (body);
+
+
+ /* bail out in case of ZERO_EXTRACT */
+ if (GET_CODE(mem) == ZERO_EXTRACT)
+ {
+ add_wild_read (bb_info);
+ insn_info->cannot_delete = true;
+ return 0;
+ }
+
/* If this is not used, then this cannot be used to keep the insn
from being deleted. On the other hand, it does provide something
Then the same pass dump looks better:
;; Function f (f)
starting the processing of deferred insns
ending the processing of deferred insns
df_analyze called
**scanning insn=5
mem: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
after cselib_expand address: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
after canon_rtx address: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
gid=0 offset=4
processing const base store gid=0[4..6)
mems_found = 1, cannot_delete = false
**scanning insn=6
mems_found = 0, cannot_delete = true
**scanning insn=7
mem: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
after cselib_expand address: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
after canon_rtx address: (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4]))
gid=0 offset=4
processing const load gid=0[4..6)
mem: (symbol_ref:HI ("g_short") <var_decl 0x2ee40b0 g_short>)
after cselib_expand address: (symbol_ref:HI ("g_short") <var_decl
0x2ee40b0 g_short>)
after canon_rtx address: (symbol_ref:HI ("g_short") <var_decl
0x2ee40b0 g_short>)
gid=1 offset=0
processing const base store gid=1[0..2)
mems_found = 1, cannot_delete = false
group 0 is frame related group 0(0+2): n p 4, 5
group 1(0+0): n p
starting the processing of deferred insns
ending the processing of deferred insns
df_analyze called
doing global processing
df_worklist_dataflow_doublequeue:n_basic_blocks 3 n_edges 2 count 5 ( 1.7)
*** Global dataflow info after analysis.
( )->[0]->( 2 )
in:
gen:
kill: *MISSING*
out: 1, 2
( 0 )->[2]->( 1 )
in: 1, 2
gen: 1, 2
kill: *MISSING*
out: 1, 2
( 2 )->[1]->( )
in: 1, 2
gen: 1, 2
kill: *MISSING*
out: *MISSING*
starting to process insn 7
v: 1, 2
i = 0, index = 0
failing at i = 0
regular read
starting to process insn 6
v:
wild read
starting to process insn 5
v:
dse: local deletions = 0, global deletions = 0, spill deletions = 0
(note 3 0 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(note 2 3 5 2 NOTE_INSN_FUNCTION_BEG)
(insn 5 2 6 2 c:\mingw\home\amirg\test\test.c:12 (set (mem/c/i:HI
(plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4])) [0 i+0 S2 A32])
(const_int 0 [0x0])) 18 {*movhi} (nil))
(insn 6 5 7 2 c:\mingw\home\amirg\test\test.c:14 (set (zero_extract:SI
(mem/s/j:QI (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4])) [0 S1 A8])
(const_int 14 [0xe])
(const_int 0 [0x0]))
(const_int 1 [0x1])) 29 {*movb_lh} (nil))
(insn 7 6 0 2 c:\mingw\home\amirg\test\test.c:15 (set (mem/c/i:HI
(symbol_ref:HI ("g_short") <var_decl 0x2ee40b0 g_short>) [0 g_short+0
S2 A16])
(mem/c/i:HI (plus:HI (reg/f:HI 35 fp)
(const_int 4 [0x4])) [0 i+0 S2 A32])) 18 {*movhi} (nil))
starting the processing of deferred insns
ending the processing of deferred insns
Amir.
On Thu, Nov 5, 2009 at 1:34 AM, Kenneth Zadeck
<Kenneth.Zadeck@naturalbridge.com> wrote:
> Amir,
>
> I tested and committed a slightly different and cleaned up version of
> this patch with a proper changelog. ?It would be good if you reported
> back the results of request by Paolo in case that shows any problems.
>
> We unfortunately cannot directly accept patches from people that do
> not have a gcc/gnu copyright assignment. ? ?We would strongly recommend
> that you go through the trouble to get this. ? It makes it much easier
> to interact with the rest of the gcc community. ?You can contact David
> Edelsohn <edelsohn@gnu.org> for details on how to do this.
>
> Thanks for the bug report and patch. ? We appreciate the help.
>
> Iant confirmed today via irc that the rtl that was generated in this private
> port was legitimate. ? I tested my patch on x86-64 to make sure that I
> did not break anything.
>
> My patch was committed as revision 153924.
>
> Kenny
>
>
>
>
>> gcc-4.4.1, proprietary backend)
>>
>> Hi,
>> It looks like fwprop does not propagate addresses that appear in
>> zero_extract on the set dest.
>> Here is an example:
>>
>> (insn 8 5 9 2 c:\mingw\home\amirg\test\test.c:157 (set (reg/f:HI 46)
>> ? ? ? ? (const:HI (plus:HI (symbol_ref:HI ("r") [flags 0x40] <var_decl
>> 0x2ee40b0 r>)
>> ? ? ? ? ? ? ? ? (const_int 68 [0x44])))) 18 {*movhi} (nil))
>>
>> (insn 9 8 0 2 c:\mingw\home\amirg\test\test.c:157 (set
>> (zero_extract:SI (mem/s/j:QI (plus:HI (reg/f:HI 46)
>> ? ? ? ? ? ? ? ? ? ? (const_int 2 [0x2])) [0 S1 A8])
>> ? ? ? ? ? ? (const_int 1 [0x1])
>> ? ? ? ? ? ? (const_int 1 [0x1]))
>> ? ? ? ? (const_int 0 [0x0])) 29 {*movb_lh} (nil))
>>
>> The problem seems to be in df_scan.c: df_uses_record doesn't handle
>> the case of (set (zero_extract(mem(...) ...) ...).
>> So I tried the following patch in df_uses_record under case SET: ...
>> case ZERO_EXTRACT: ...
>>
>> --- df-scan.c
>> +++ df-scan.c
>> @@ -3152,11 +3152,24 @@
>> ? ? ? ? ? ? ? ? {
>> ? ? ? ? ? ? ? ? ? width = INTVAL (XEXP (dst, 1));
>> ? ? ? ? ? ? ? ? ? offset = INTVAL (XEXP (dst, 2));
>> - ? ? ? ? ? ? ? ? mode = GET_MODE (dst);
>> + ? ? ? ? ? ? ? ? mode = GET_MODE (dst);
>> + ? ? ? ? ? ? ? ? ? ? /* AG start */
>> + ? ? ? ? ? ? ? ? ? ? /* Handle the case of zero_extract(mem(...)) in the set dest */
>> + ? ? ? ? ? ? ? ? ? ? if (GET_CODE (XEXP (dst,0)) == MEM)
>> + ? ? ? ? ? ? ? ? ? ? {
>> + ? ? ? ? ? ? ? ? ? ? df_uses_record (DF_REF_EXTRACT, collection_rec, &XEXP (XEXP (dst,0), 0),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DF_REF_REG_MEM_STORE, bb, insn_info,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DF_REF_ZERO_EXTRACT,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? width, offset, mode);
>> + ? ? ? ? ? ? ? ? ? ? }
>> + ? ? ? ? ? ? ? ? ? ? else
>> + ? ? ? ? ? ? ? ? ? ? {
>> + ? ? ? ? ? ? ? ? ? ? /* AG end */
>> ? ? ? ? ? ? ? ? ? ? ? df_uses_record (DF_REF_EXTRACT, collection_rec, &XEXP (dst, 0),
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DF_REF_REG_USE, bb, insn_info,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DF_REF_READ_WRITE | DF_REF_ZERO_EXTRACT,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? width, offset, mode);
>> + ? ? ? ? ? ? ? ? ? ? } /* AG added closing brackets */
>> ? ? ? ? ? ? ? ? }
>> ? ? ? ? ? ? ? else
>> ? ? ? ? ? ? ? ? {
>>
>> and now fwprop propagates properly:
>>
>> (insn 9 5 0 2 c:\mingw\home\amirg\test\test.c:157 (set
>> (zero_extract:SI (mem/s/j:QI (const:HI (plus:HI (symbol_ref:HI ("r")
>> [flags 0x40] <var_decl 0x2ee40b0 r>)
>> ? ? ? ? ? ? ? ? ? ? ? ? (const_int 70 [0x46]))) [0 S1 A8])
>> ? ? ? ? ? ? (const_int 1 [0x1])
>> ? ? ? ? ? ? (const_int 1 [0x1]))
>> ? ? ? ? (const_int 0 [0x0])) 29 {*movb_lh} (nil))
>>
>> I'll appreciate any comments,
>> Thanks,
>>
>> Amir
>>
>
> ===File ~/gcc/patches/dataflow/scanfix2.diff================
> Index: gcc/df-scan.c
> ===================================================================
> --- gcc/df-scan.c ? ? ? (revision 153920)
> +++ gcc/df-scan.c ? ? ? (working copy)
> @@ -3248,10 +3248,23 @@ df_uses_record (enum df_ref_class cl, st
> ? ? ? ? ? ? ? ? ? ?width = INTVAL (XEXP (dst, 1));
> ? ? ? ? ? ? ? ? ? ?offset = INTVAL (XEXP (dst, 2));
> ? ? ? ? ? ? ? ? ? ?mode = GET_MODE (dst);
> - ? ? ? ? ? ? ? ? ? df_uses_record (DF_REF_EXTRACT, collection_rec, &XEXP (dst, 0),
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DF_REF_REG_USE, bb, insn_info,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DF_REF_READ_WRITE | DF_REF_ZERO_EXTRACT,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? width, offset, mode);
> + ? ? ? ? ? ? ? ? ? if (GET_CODE (XEXP (dst,0)) == MEM)
> + ? ? ? ? ? ? ? ? ? ? {
> + ? ? ? ? ? ? ? ? ? ? ? /* Handle the case of zero_extract(mem(...)) in the set dest.
> + ? ? ? ? ? ? ? ? ? ? ? ? ?This special case is allowed only if the mem is a single byte and
> + ? ? ? ? ? ? ? ? ? ? ? ? ?is useful to set a bitfield in memory. ?*/
> + ? ? ? ? ? ? ? ? ? ? ? df_uses_record (DF_REF_EXTRACT, collection_rec, &XEXP (XEXP (dst,0), 0),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DF_REF_REG_MEM_STORE, bb, insn_info,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DF_REF_ZERO_EXTRACT,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? width, offset, mode);
> + ? ? ? ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? {
> + ? ? ? ? ? ? ? ? ? ? ? df_uses_record (DF_REF_EXTRACT, collection_rec, &XEXP (dst, 0),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DF_REF_REG_USE, bb, insn_info,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DF_REF_READ_WRITE | DF_REF_ZERO_EXTRACT,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? width, offset, mode);
> + ? ? ? ? ? ? ? ? ? ? }
> ? ? ? ? ? ? ? ? ?}
> ? ? ? ? ? ? ? ?else
> ? ? ? ? ? ? ? ? ?{
> ============================================================
>