* PATCH: PR target/49142: Invalid 8bit register operand @ 2011-05-24 17:35 H.J. Lu 2011-05-25 14:31 ` Uros Bizjak 0 siblings, 1 reply; 7+ messages in thread From: H.J. Lu @ 2011-05-24 17:35 UTC (permalink / raw) To: gcc-patches; +Cc: Uros Bizjak Hi, We are working on a new optimization, which turns off TARGET_MOVX. GCC generates: movb %ah, %dil But %ah can only be used with %[abcd][hl]. This patch adds QIreg_operand and uses it in *movqi_extv_1_rex64/*movqi_extzv_2_rex64. OK for trunk if there is no regression? Thanks. H.J. --- 2011-05-24 H.J. Lu <hongjiu.lu@intel.com> PR target/49142 * config/i386/i386.md (*movqi_extv_1_rex64): Replace q_regs_operand with QIreg_operand. (*movqi_extzv_2_rex64): Likewise. * config/i386/predicates.md (QIreg_operand): New. diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 6c158cf..b7f5866 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -2635,7 +2635,7 @@ } [(set (attr "type") (if_then_else (and (match_operand:QI 0 "register_operand" "") - (ior (not (match_operand:QI 0 "q_regs_operand" "")) + (ior (not (match_operand:QI 0 "QIreg_operand" "")) (ne (symbol_ref "TARGET_MOVX") (const_int 0)))) (const_string "imovx") @@ -2699,7 +2699,7 @@ } } [(set (attr "type") - (if_then_else (ior (not (match_operand:QI 0 "q_regs_operand" "")) + (if_then_else (ior (not (match_operand:QI 0 "QIreg_operand" "")) (ne (symbol_ref "TARGET_MOVX") (const_int 0))) (const_string "imovx") diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md index 8a89f70..1471f5a 100644 --- a/gcc/config/i386/predicates.md +++ b/gcc/config/i386/predicates.md @@ -82,6 +82,10 @@ (and (match_code "reg") (match_test "REGNO (op) == FLAGS_REG"))) +;; Return true if op is one of QImode registers: %[abcd][hl]. +(define_predicate "QIreg_operand" + (match_test "QI_REG_P (op)")) + ;; Return true if op is a QImode register operand other than ;; %[abcd][hl]. (define_predicate "ext_QIreg_operand" ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: PR target/49142: Invalid 8bit register operand 2011-05-24 17:35 PATCH: PR target/49142: Invalid 8bit register operand H.J. Lu @ 2011-05-25 14:31 ` Uros Bizjak 2011-05-25 15:10 ` H.J. Lu 0 siblings, 1 reply; 7+ messages in thread From: Uros Bizjak @ 2011-05-25 14:31 UTC (permalink / raw) To: H.J. Lu; +Cc: gcc-patches On Tue, May 24, 2011 at 5:54 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: > Hi, > > We are working on a new optimization, which turns off TARGET_MOVX. > GCC generates: > > movb %ah, %dil > > But %ah can only be used with %[abcd][hl]. This patch adds QIreg_operand > and uses it in *movqi_extv_1_rex64/*movqi_extzv_2_rex64. OK for trunk > if there is no regression? If this is the case, then please change "q_regs_operand" predicate to accept just QI_REG_P registers. Uros. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: PR target/49142: Invalid 8bit register operand 2011-05-25 14:31 ` Uros Bizjak @ 2011-05-25 15:10 ` H.J. Lu 2011-05-25 16:44 ` Uros Bizjak 0 siblings, 1 reply; 7+ messages in thread From: H.J. Lu @ 2011-05-25 15:10 UTC (permalink / raw) To: Uros Bizjak; +Cc: gcc-patches On Wed, May 25, 2011 at 7:00 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Tue, May 24, 2011 at 5:54 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >> Hi, >> >> We are working on a new optimization, which turns off TARGET_MOVX. >> GCC generates: >> >> movb %ah, %dil >> >> But %ah can only be used with %[abcd][hl]. This patch adds QIreg_operand >> and uses it in *movqi_extv_1_rex64/*movqi_extzv_2_rex64. OK for trunk >> if there is no regression? > > If this is the case, then please change "q_regs_operand" predicate to > accept just QI_REG_P registers. > I thought about it. It is a problem only with %[abcd]h. I am not sure if changing q_regs_operand to accept just QI_REG_P registers will negatively impact (define_peephole2 [(set (reg FLAGS_REG) (match_operand 0 "" "")) (set (match_operand:QI 1 "register_operand" "") (match_operator:QI 2 "ix86_comparison_operator" [(reg FLAGS_REG) (const_int 0)])) (set (match_operand 3 "q_regs_operand" "") (zero_extend (match_dup 1)))] "(peep2_reg_dead_p (3, operands[1]) || operands_match_p (operands[1], operands[3])) && ! reg_overlap_mentioned_p (operands[3], operands[0])" [(set (match_dup 4) (match_dup 0)) (set (strict_low_part (match_dup 5)) (match_dup 2))] (define_peephole2 [(set (reg FLAGS_REG) (match_operand 0 "" "")) (set (match_operand:QI 1 "register_operand" "") (match_operator:QI 2 "ix86_comparison_operator" [(reg FLAGS_REG) (const_int 0)])) (parallel [(set (match_operand 3 "q_regs_operand" "") (zero_extend (match_dup 1))) (clobber (reg:CC FLAGS_REG))])] "(peep2_reg_dead_p (3, operands[1]) || operands_match_p (operands[1], operands[3])) && ! reg_overlap_mentioned_p (operands[3], operands[0])" [(set (match_dup 4) (match_dup 0)) (set (strict_low_part (match_dup 5)) (match_dup 2))] -- H.J. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: PR target/49142: Invalid 8bit register operand 2011-05-25 15:10 ` H.J. Lu @ 2011-05-25 16:44 ` Uros Bizjak 2011-05-25 17:26 ` H.J. Lu 0 siblings, 1 reply; 7+ messages in thread From: Uros Bizjak @ 2011-05-25 16:44 UTC (permalink / raw) To: H.J. Lu; +Cc: gcc-patches On Wed, May 25, 2011 at 4:42 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, May 25, 2011 at 7:00 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Tue, May 24, 2011 at 5:54 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>> Hi, >>> >>> We are working on a new optimization, which turns off TARGET_MOVX. >>> GCC generates: >>> >>> movb %ah, %dil >>> >>> But %ah can only be used with %[abcd][hl]. This patch adds QIreg_operand >>> and uses it in *movqi_extv_1_rex64/*movqi_extzv_2_rex64. OK for trunk >>> if there is no regression? and Replace q_regs_operand with QIreg_operand. ( >> >> If this is the case, then please change "q_regs_operand" predicate to >> accept just QI_REG_P registers. >> > > I thought about it. It is a problem only with %[abcd]h. I am not sure if > changing q_regs_operand to accept just QI_REG_P registers will negatively > impact I see. The patch is OK then, but for consistency, please change the predicate of *movqi_extv_1*movqi_extzv_2 as well. Oh, and the "register_operand" check in "type" calculation can be removed. Thanks, Uros. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: PR target/49142: Invalid 8bit register operand 2011-05-25 16:44 ` Uros Bizjak @ 2011-05-25 17:26 ` H.J. Lu 2011-05-25 19:46 ` Uros Bizjak 0 siblings, 1 reply; 7+ messages in thread From: H.J. Lu @ 2011-05-25 17:26 UTC (permalink / raw) To: Uros Bizjak; +Cc: gcc-patches On Wed, May 25, 2011 at 8:30 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Wed, May 25, 2011 at 4:42 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Wed, May 25, 2011 at 7:00 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >>> On Tue, May 24, 2011 at 5:54 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >>>> Hi, >>>> >>>> We are working on a new optimization, which turns off TARGET_MOVX. >>>> GCC generates: >>>> >>>> movb %ah, %dil >>>> >>>> But %ah can only be used with %[abcd][hl]. This patch adds QIreg_operand >>>> and uses it in *movqi_extv_1_rex64/*movqi_extzv_2_rex64. OK for trunk >>>> if there is no regression? and Replace > q_regs_operand with QIreg_operand. > ( >>> >>> If this is the case, then please change "q_regs_operand" predicate to >>> accept just QI_REG_P registers. >>> >> >> I thought about it. It is a problem only with %[abcd]h. I am not sure if >> changing q_regs_operand to accept just QI_REG_P registers will negatively >> impact > > I see. The patch is OK then, but for consistency, please change the > predicate of *movqi_extv_1*movqi_extzv_2 as well. Oh, and the > "register_operand" check in "type" calculation can be removed. > > Thanks, > Uros. > This is what I checked in. Thanks. -- H.J. --- 2011-05-25 H.J. Lu <hongjiu.lu@intel.com> PR target/49142 * config/i386/i386.md (*movqi_extv_1_rex64): Remove "register_operand" check and replace q_regs_operand with QIreg_operand in "type" calculation. (*movqi_extv_1): Likewise. (*movqi_extzv_2_rex64): Likewise. (*movqi_extzv_2): Likewise. * config/i386/predicates.md (QIreg_operand): New. diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 49f1ee7..3b59024 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -2487,10 +2487,9 @@ } } [(set (attr "type") - (if_then_else (and (match_operand:QI 0 "register_operand" "") - (ior (not (match_operand:QI 0 "q_regs_operand" "")) - (ne (symbol_ref "TARGET_MOVX") - (const_int 0)))) + (if_then_else (ior (not (match_operand:QI 0 "QIreg_operand" "")) + (ne (symbol_ref "TARGET_MOVX") + (const_int 0))) (const_string "imovx") (const_string "imov"))) (set (attr "mode") @@ -2514,10 +2513,9 @@ } } [(set (attr "type") - (if_then_else (and (match_operand:QI 0 "register_operand" "") - (ior (not (match_operand:QI 0 "q_regs_operand" "")) - (ne (symbol_ref "TARGET_MOVX") - (const_int 0)))) + (if_then_else (ior (not (match_operand:QI 0 "QIreg_operand" "")) + (ne (symbol_ref "TARGET_MOVX") + (const_int 0))) (const_string "imovx") (const_string "imov"))) (set (attr "mode") @@ -2552,7 +2550,7 @@ } } [(set (attr "type") - (if_then_else (ior (not (match_operand:QI 0 "q_regs_operand" "")) + (if_then_else (ior (not (match_operand:QI 0 "QIreg_operand" "")) (ne (symbol_ref "TARGET_MOVX") (const_int 0))) (const_string "imovx") @@ -2579,10 +2577,9 @@ } } [(set (attr "type") - (if_then_else (and (match_operand:QI 0 "register_operand" "") - (ior (not (match_operand:QI 0 "q_regs_operand" "")) - (ne (symbol_ref "TARGET_MOVX") - (const_int 0)))) + (if_then_else (ior (not (match_operand:QI 0 "QIreg_operand" "")) + (ne (symbol_ref "TARGET_MOVX") + (const_int 0))) (const_string "imovx") (const_string "imov"))) (set (attr "mode") diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md index 8a89f70..1471f5a 100644 --- a/gcc/config/i386/predicates.md +++ b/gcc/config/i386/predicates.md @@ -82,6 +82,10 @@ (and (match_code "reg") (match_test "REGNO (op) == FLAGS_REG"))) +;; Return true if op is one of QImode registers: %[abcd][hl]. +(define_predicate "QIreg_operand" + (match_test "QI_REG_P (op)")) + ;; Return true if op is a QImode register operand other than ;; %[abcd][hl]. (define_predicate "ext_QIreg_operand" ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: PR target/49142: Invalid 8bit register operand 2011-05-25 17:26 ` H.J. Lu @ 2011-05-25 19:46 ` Uros Bizjak 2011-05-25 20:01 ` H.J. Lu 0 siblings, 1 reply; 7+ messages in thread From: Uros Bizjak @ 2011-05-25 19:46 UTC (permalink / raw) To: H.J. Lu; +Cc: gcc-patches On Wed, May 25, 2011 at 6:20 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>> We are working on a new optimization, which turns off TARGET_MOVX. >>>>> GCC generates: >>>>> >>>>> movb %ah, %dil >>>>> >>>>> But %ah can only be used with %[abcd][hl]. This patch adds QIreg_operand >>>>> and uses it in *movqi_extv_1_rex64/*movqi_extzv_2_rex64. OK for trunk >>>>> if there is no regression? and Replace >> q_regs_operand with QIreg_operand. >> ( >>>> >>>> If this is the case, then please change "q_regs_operand" predicate to >>>> accept just QI_REG_P registers. >>>> >>> >>> I thought about it. It is a problem only with %[abcd]h. I am not sure if >>> changing q_regs_operand to accept just QI_REG_P registers will negatively >>> impact >> >> I see. The patch is OK then, but for consistency, please change the >> predicate of *movqi_extv_1*movqi_extzv_2 as well. Oh, and the >> "register_operand" check in "type" calculation can be removed. >> >> Thanks, >> Uros. >> > > This is what I checked in. > > Thanks. > > -- > H.J. > --- > 2011-05-25 H.J. Lu <hongjiu.lu@intel.com> > > PR target/49142 > * config/i386/i386.md (*movqi_extv_1_rex64): Remove > "register_operand" check and replace q_regs_operand with > QIreg_operand in "type" calculation. > (*movqi_extv_1): Likewise. > (*movqi_extzv_2_rex64): Likewise. > (*movqi_extzv_2): Likewise. Er, I didn't mean to remove register_operand check from 32bit patterns... there, operand 0 can also be memory operand due to "nonimmediate_operand" constraint. Uros. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PATCH: PR target/49142: Invalid 8bit register operand 2011-05-25 19:46 ` Uros Bizjak @ 2011-05-25 20:01 ` H.J. Lu 0 siblings, 0 replies; 7+ messages in thread From: H.J. Lu @ 2011-05-25 20:01 UTC (permalink / raw) To: Uros Bizjak; +Cc: gcc-patches On Wed, May 25, 2011 at 12:11 PM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Wed, May 25, 2011 at 6:20 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > >>>>>> We are working on a new optimization, which turns off TARGET_MOVX. >>>>>> GCC generates: >>>>>> >>>>>> movb %ah, %dil >>>>>> >>>>>> But %ah can only be used with %[abcd][hl]. This patch adds QIreg_operand >>>>>> and uses it in *movqi_extv_1_rex64/*movqi_extzv_2_rex64. OK for trunk >>>>>> if there is no regression? and Replace >>> q_regs_operand with QIreg_operand. >>> ( >>>>> >>>>> If this is the case, then please change "q_regs_operand" predicate to >>>>> accept just QI_REG_P registers. >>>>> >>>> >>>> I thought about it. It is a problem only with %[abcd]h. I am not sure if >>>> changing q_regs_operand to accept just QI_REG_P registers will negatively >>>> impact >>> >>> I see. The patch is OK then, but for consistency, please change the >>> predicate of *movqi_extv_1*movqi_extzv_2 as well. Oh, and the >>> "register_operand" check in "type" calculation can be removed. >>> >>> Thanks, >>> Uros. >>> >> >> This is what I checked in. >> >> Thanks. >> >> -- >> H.J. >> --- >> 2011-05-25 H.J. Lu <hongjiu.lu@intel.com> >> >> PR target/49142 >> * config/i386/i386.md (*movqi_extv_1_rex64): Remove >> "register_operand" check and replace q_regs_operand with >> QIreg_operand in "type" calculation. >> (*movqi_extv_1): Likewise. >> (*movqi_extzv_2_rex64): Likewise. >> (*movqi_extzv_2): Likewise. > > Er, I didn't mean to remove register_operand check from 32bit > patterns... there, operand 0 can also be memory operand due to > "nonimmediate_operand" constraint. Ooops. I am checking in this. Thanks. -- H.J. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index ed1834f..1afef8e 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,11 @@ 2011-05-25 H.J. Lu <hongjiu.lu@intel.com> + * config/i386/i386.md (*movqi_extv_1)): Put back + "register_operand" check in "type" calculation. + (*movqi_extzv_2): Likewise. + +2011-05-25 H.J. Lu <hongjiu.lu@intel.com> + * doc/extend.texi (X86 Built-in Functions): Update pause intrinsic. diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 1cdbe7e..13a1cde 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -2514,9 +2514,10 @@ } } [(set (attr "type") - (if_then_else (ior (not (match_operand:QI 0 "QIreg_operand" "")) - (ne (symbol_ref "TARGET_MOVX") - (const_int 0))) + (if_then_else (and (match_operand:QI 0 "register_operand" "") + (ior (not (match_operand:QI 0 "QIreg_operand" "")) + (ne (symbol_ref "TARGET_MOVX") + (const_int 0)))) (const_string "imovx") (const_string "imov"))) (set (attr "mode") @@ -2578,9 +2579,10 @@ } } [(set (attr "type") - (if_then_else (ior (not (match_operand:QI 0 "QIreg_operand" "")) - (ne (symbol_ref "TARGET_MOVX") - (const_int 0))) + (if_then_else (and (match_operand:QI 0 "register_operand" "") + (ior (not (match_operand:QI 0 "QIreg_operand" "")) + (ne (symbol_ref "TARGET_MOVX") + (const_int 0)))) (const_string "imovx") (const_string "imov"))) (set (attr "mode") ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-05-25 19:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-05-24 17:35 PATCH: PR target/49142: Invalid 8bit register operand H.J. Lu 2011-05-25 14:31 ` Uros Bizjak 2011-05-25 15:10 ` H.J. Lu 2011-05-25 16:44 ` Uros Bizjak 2011-05-25 17:26 ` H.J. Lu 2011-05-25 19:46 ` Uros Bizjak 2011-05-25 20:01 ` H.J. Lu
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).