| Message ID | CAFULd4Z2=GmtJAB4YZxfmtDZGCVCpNWWycjOngHkTM6M5A5J3Q@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 02/07/2012 08:37 AM, Uros Bizjak wrote: > On Tue, Feb 7, 2012 at 1:59 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > >> Attached patch declares CCZmode compatible with CCGOC, CCGO and CCNO modes. > > Actually, CCZ mode is not compatible with CCNO mode, since the later > only declares that overflow flag is not set. CCGOC and CCGO declare > garbage in overflow (and carry in case of CCGOC) flag, so implicitly > declare that CCZ flag is valid. Following this reasoning, CCZ mode > should be compatible with CCGOC and CCGO modes. We should probably fix this confusion by renaming the modes so that they're all positive: CCNO -> CCCSZ CCGC -> CCOSZ CCGOC -> CCSZ r~
On Tue, Feb 7, 2012 at 5:57 PM, Richard Henderson <rth@redhat.com> wrote: >>> Attached patch declares CCZmode compatible with CCGOC, CCGO and CCNO modes. >> >> Actually, CCZ mode is not compatible with CCNO mode, since the later >> only declares that overflow flag is not set. CCGOC and CCGO declare >> garbage in overflow (and carry in case of CCGOC) flag, so implicitly >> declare that CCZ flag is valid. Following this reasoning, CCZ mode >> should be compatible with CCGOC and CCGO modes. > > We should probably fix this confusion by renaming the modes so that > they're all positive: > > CCNO -> CCCSZ > CCGC -> CCOSZ > CCGOC -> CCSZ No, no. Once you get the logic, it actually makes sense to name them that way. Regarding your proposed change, CCNO does not say that CSZ are all valid, it says that OF = 0, so the first line is wrong. Uros.
On Tue, Feb 7, 2012 at 5:37 PM, Uros Bizjak <ubizjak@gmail.com> wrote: >> Attached patch declares CCZmode compatible with CCGOC, CCGO and CCNO modes. > > Actually, CCZ mode is not compatible with CCNO mode, since the later > only declares that overflow flag is not set. CCGOC and CCGO declare > garbage in overflow (and carry in case of CCGOC) flag, so implicitly > declare that CCZ flag is valid. Following this reasoning, CCZ mode > should be compatible with CCGOC and CCGO modes. > > 2012-02-07 Uros Bizjak <ubizjak@gmail.com> > > * config/i386/i386.c (ix86_cc_modes_compatible): Declare CCZmode > compatible with CCGOCmode and CCGCmode. > > Attached patch was bootstrapped and regression tested on x86_64-pc-linux-gnu. ... where it uncovers another problem how RTL optimizer handles compatible compares! With attached patch, the example from pr28685 --cut here-- int test(int a, int b) { int lt = a < b; int eq = a == b; if (lt) return 1; return eq; } --cut here-- triggers compare elimination in CSE2 pass. We enter CSE2 pass with: (insn 8 5 9 2 (set (reg:CCGC 17 flags) (compare:CCGC (reg/v:SI 62 [ a ]) (reg/v:SI 63 [ b ]))) pr28685.c:7 6 {*cmpsi_1} (nil)) (jump_insn 9 8 10 2 (set (pc) (if_then_else (lt (reg:CCGC 17 flags) (const_int 0 [0])) (label_ref:DI 15) (pc))) pr28685.c:7 599 {*jcc_1} (expr_list:REG_DEAD (reg:CCGC 17 flags) (expr_list:REG_BR_PROB (const_int 3900 [0xf3c]) (nil))) -> 15) [...] (note 10 9 11 3 [bb 3] NOTE_INSN_BASIC_BLOCK) (insn 11 10 12 3 (set (reg:CCZ 17 flags) (compare:CCZ (reg/v:SI 62 [ a ]) (reg/v:SI 63 [ b ]))) pr28685.c:6 6 {*cmpsi_1} (expr_list:REG_DEAD (reg/v:SI 63 [ b ]) (expr_list:REG_DEAD (reg/v:SI 62 [ a ]) (nil)))) (insn 12 11 13 3 (set (reg:QI 65) (eq:QI (reg:CCZ 17 flags) (const_int 0 [0]))) pr28685.c:6 595 {*setcc_qi} (expr_list:REG_DEAD (reg:CCZ 17 flags) (nil))) After CSE2 pass, we have: (insn 8 5 9 2 (set (reg:CCGC 17 flags) (compare:CCGC (reg/v:SI 62 [ a ]) (reg/v:SI 63 [ b ]))) pr28685.c:7 6 {*cmpsi_1} (nil)) (jump_insn 9 8 10 2 (set (pc) (if_then_else (lt (reg:CCGC 17 flags) (const_int 0 [0])) (label_ref:DI 15) (pc))) pr28685.c:7 599 {*jcc_1} (expr_list:REG_DEAD (reg:CCGC 17 flags) (expr_list:REG_BR_PROB (const_int 3900 [0xf3c]) (nil))) -> 15) [...] (note 10 9 12 3 [bb 3] NOTE_INSN_BASIC_BLOCK) (insn 12 10 13 3 (set (reg:QI 65) (eq:QI (reg:CCGC 17 flags) (const_int 0 [0]))) pr28685.c:6 595 {*setcc_qi} (expr_list:REG_DEAD (reg:CCGC 17 flags) (nil))) CSE2 pass eliminated (insn 11), which is OK since CCZ is compatible with CCGC, so (CCGC, CCZ)->CCGC. However, the pass also changed the mode of flags register in (insn 12). I don't think this is correct, the user should not be changed at all. Uros.
On Sat, Feb 11, 2012 at 9:41 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>> Attached patch declares CCZmode compatible with CCGOC, CCGO and CCNO modes. >> >> Actually, CCZ mode is not compatible with CCNO mode, since the later >> only declares that overflow flag is not set. CCGOC and CCGO declare >> garbage in overflow (and carry in case of CCGOC) flag, so implicitly >> declare that CCZ flag is valid. Following this reasoning, CCZ mode >> should be compatible with CCGOC and CCGO modes. >> >> 2012-02-07 Uros Bizjak <ubizjak@gmail.com> >> >> * config/i386/i386.c (ix86_cc_modes_compatible): Declare CCZmode >> compatible with CCGOCmode and CCGCmode. >> >> Attached patch was bootstrapped and regression tested on x86_64-pc-linux-gnu. > > ... where it uncovers another problem how RTL optimizer handles > compatible compares! > CSE2 pass eliminated (insn 11), which is OK since CCZ is compatible > with CCGC, so (CCGC, CCZ)->CCGC. However, the pass also changed the > mode of flags register in (insn 12). I don't think this is correct, > the user should not be changed at all. This is also where my proposed patch [1] for post-reload compare elimination differs from CSE2 pass. The proposed revision doesn't update the mode of flags reg of users to calculated compatible mode. FWIW, the mode of flags in users doesn't matter at all on x86, but which way is correct? [1] http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00466.html Uros.
On 02/11/2012 12:56 AM, Uros Bizjak wrote: > FWIW, the mode of flags in users doesn't matter at all on x86, but > which way is correct? As far as I know, it doesn't matter anywhere. We don't even bother to have perfect harmony between integer modes in hard registers -- think about what happens when we drop all the subregs on the floor post-reload. Yes, it's probably an error if we don't have compatible modes between def and use, but nothing is going to check for that. r~
On Tue, Feb 14, 2012 at 12:00 AM, Richard Henderson <rth@redhat.com> wrote: > On 02/11/2012 12:56 AM, Uros Bizjak wrote: >> FWIW, the mode of flags in users doesn't matter at all on x86, but >> which way is correct? > > As far as I know, it doesn't matter anywhere. We don't even bother to have perfect harmony between integer modes in hard registers -- think about what happens when we drop all the subregs on the floor post-reload. Yes, it's probably an error if we don't have compatible modes between def and use, but nothing is going to check for that. cse.c says some relaxing words related to this issue: /* If the following assertion was triggered, there is most probably something wrong with the cc_modes_compatible back end function. CC modes only can be considered compatible if the insn - with the mode replaced by any of the compatible modes - can still be recognized. */ It looks to me that correct definition of cc_modes_compatible guarantees that insn is still valid, no matter if the mode of flags remains in the "wrong mode". In any case, I will add the comment to avoid confusion. Thanks, Uros.
Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 183968) +++ config/i386/i386.c (working copy) @@ -17778,6 +17778,11 @@ ix86_cc_modes_compatible (enum machine_mode m1, en || (m1 == CCGOCmode && m2 == CCGCmode)) return CCGCmode; + if (m1 == CCZmode && (m2 == CCGCmode || m2 == CCGOCmode)) + return m2; + else if (m2 == CCZmode && (m1 == CCGCmode || m1 == CCGOCmode)) + return m1; + switch (m1) { default:
On Tue, Feb 7, 2012 at 1:59 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > Attached patch declares CCZmode compatible with CCGOC, CCGO and CCNO modes. Actually, CCZ mode is not compatible with CCNO mode, since the later only declares that overflow flag is not set. CCGOC and CCGO declare garbage in overflow (and carry in case of CCGOC) flag, so implicitly declare that CCZ flag is valid. Following this reasoning, CCZ mode should be compatible with CCGOC and CCGO modes. 2012-02-07 Uros Bizjak <ubizjak@gmail.com> * config/i386/i386.c (ix86_cc_modes_compatible): Declare CCZmode compatible with CCGOCmode and CCGCmode. Attached patch was bootstrapped and regression tested on x86_64-pc-linux-gnu. Uros.