From patchwork Thu Jul 1 17:00:16 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 57563 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 518B11007D1 for ; Fri, 2 Jul 2010 03:08:36 +1000 (EST) Received: (qmail 5424 invoked by alias); 1 Jul 2010 17:08:33 -0000 Received: (qmail 5411 invoked by uid 22791); 1 Jul 2010 17:08:31 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, TW_DQ, TW_IV X-Spam-Check-By: sourceware.org Received: from mail-pz0-f47.google.com (HELO mail-pz0-f47.google.com) (209.85.210.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 01 Jul 2010 17:08:26 +0000 Received: by pzk4 with SMTP id 4so346030pzk.20 for ; Thu, 01 Jul 2010 10:08:24 -0700 (PDT) MIME-Version: 1.0 Received: by 10.142.209.15 with SMTP id h15mr12843174wfg.150.1278003616433; Thu, 01 Jul 2010 10:00:16 -0700 (PDT) Received: by 10.142.231.8 with HTTP; Thu, 1 Jul 2010 10:00:16 -0700 (PDT) In-Reply-To: <4C2CA3C6.8080608@gnu.org> References: <20100629191246.GA2770@intel.com> <4C2CA3C6.8080608@gnu.org> Date: Thu, 1 Jul 2010 10:00:16 -0700 Message-ID: Subject: Re: RFC: PR rtl-optimization/44695: [4.6 Regression] ice in simplify_subreg, at simplify-rtx.c:5117 From: "H.J. Lu" To: Paolo Bonzini Cc: gcc-patches@gcc.gnu.org X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Thu, Jul 1, 2010 at 7:18 AM, Paolo Bonzini wrote: >> Why isn't HImode on operand 1 OK? > > Because modes don't match between the div and its operands. > >>> My comment in the PR is based on how GCC handles extension of the >>> dividend for 16/32/64 bit operands. >>> >> >> Your suggestion generate a pattern unsupported by hardware >> before reload > > That's what 16/32/64 bit operands do too > > (define_expand "divmod4" >  [(parallel [(set (match_operand:SWIM248 0 "register_operand" "") >                   (div:SWIM248 >                     (match_operand:SWIM248 1 "register_operand" "") >                     (match_operand:SWIM248 2 "nonimmediate_operand" ""))) >              (set (match_operand:SWIM248 3 "register_operand" "") >                   (mod:SWIM248 (match_dup 1) (match_dup 2))) >              (clobber (reg:CC FLAGS_REG))])] >  "" >  "") For 16/32/64 bit divide, since dividend is in AX/DX, we can set DX after reload. For 8bit, dividend is in AX. We want to set dividend properly before reload to avoid extra extension. > >> and introduces an extra register move during split to fix it. > > I think you mean one extra extension?  That's possible, though it could be Yes. > fixed in combine using nonzero_bits or num_sign_bit_copies. > > Anyway, can you please post the patch and the result?  I think you should go > one step at a time and first fix this bug, then optimize it. > While working on it, I noticed that divmodqi4 had mod, which is wrong for unsigned divmod. There should be divmodqi4 and udivmodqi4. This patch fixes it. It also uses subreg:QI on operand 1 in divmodhiqi3 and udivmodhiqi3. Does it look OK? Thanks. diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 1714d50..62aee26 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -760,8 +760,6 @@ ;; Used in signed and unsigned divisions. (define_code_iterator any_div [div udiv]) -(define_code_attr extract_code - [(div "SIGN_EXTRACT") (udiv "ZERO_EXTRACT")]) ;; Instruction prefix for signed and unsigned operations. (define_code_attr sgnprefix [(sign_extend "i") (zero_extend "") @@ -7308,9 +7306,9 @@ ;; Divmod instructions. -(define_expand "divmodqi4" +(define_expand "divmodqi4" [(parallel [(set (match_operand:QI 0 "register_operand" "") - (any_div:QI + (div:QI (match_operand:QI 1 "register_operand" "") (match_operand:QI 2 "nonimmediate_operand" ""))) (set (match_operand:QI 3 "register_operand" "") @@ -7326,31 +7324,51 @@ /* Extend operands[1] to HImode. Generate 8bit divide. Result is in AX. */ - if ( == SIGN_EXTRACT) - { - emit_insn (gen_extendqihi2 (tmp1, operands[1])); - emit_insn (gen_divmodhiqi3 (tmp0, tmp1, operands[2])); + emit_insn (gen_extendqihi2 (tmp1, operands[1])); + emit_insn (gen_divmodhiqi3 (tmp0, tmp1, operands[2])); - div = gen_rtx_DIV (QImode, operands[1], operands[2]); - mod = gen_rtx_MOD (QImode, operands[1], operands[2]); + div = gen_rtx_DIV (QImode, operands[1], operands[2]); + mod = gen_rtx_MOD (QImode, operands[1], operands[2]); - tmp1 = gen_rtx_ (QImode, tmp0, - GEN_INT (8), GEN_INT (8)); - } - else - { - emit_insn (gen_zero_extendqihi2 (tmp1, operands[1])); - emit_insn (gen_udivmodhiqi3 (tmp0, tmp1, operands[2])); + /* Extract remainder from AH. */ + tmp1 = gen_rtx_SIGN_EXTRACT (QImode, tmp0, GEN_INT (8), GEN_INT (8)); + insn = emit_move_insn (operands[3], tmp1); + set_unique_reg_note (insn, REG_EQUAL, mod); - div = gen_rtx_UDIV (QImode, operands[1], operands[2]); - mod = gen_rtx_UMOD (QImode, operands[1], operands[2]); + /* Extract quotient from AL. */ + insn = emit_move_insn (operands[0], gen_lowpart (QImode, tmp0)); + set_unique_reg_note (insn, REG_EQUAL, div); - tmp1 = gen_rtx_ (SImode, tmp0, - GEN_INT (8), GEN_INT (8)); - tmp1 = simplify_gen_subreg (QImode, tmp1, SImode, 0); - } + DONE; +}) + +(define_expand "udivmodqi4" + [(parallel [(set (match_operand:QI 0 "register_operand" "") + (udiv:QI + (match_operand:QI 1 "register_operand" "") + (match_operand:QI 2 "nonimmediate_operand" ""))) + (set (match_operand:QI 3 "register_operand" "") + (umod:QI (match_dup 1) (match_dup 2))) + (clobber (reg:CC FLAGS_REG))])] + "TARGET_QIMODE_MATH" +{ + rtx div, mod, insn; + rtx tmp0, tmp1; + + tmp0 = gen_reg_rtx (HImode); + tmp1 = gen_reg_rtx (HImode); + + /* Extend operands[1] to HImode. Generate 8bit divide. Result is + in AX. */ + emit_insn (gen_zero_extendqihi2 (tmp1, operands[1])); + emit_insn (gen_udivmodhiqi3 (tmp0, tmp1, operands[2])); + + div = gen_rtx_UDIV (QImode, operands[1], operands[2]); + mod = gen_rtx_UMOD (QImode, operands[1], operands[2]); /* Extract remainder from AH. */ + tmp1 = gen_rtx_ZERO_EXTRACT (SImode, tmp0, GEN_INT (8), GEN_INT (8)); + tmp1 = simplify_gen_subreg (QImode, tmp1, SImode, 0); insn = emit_move_insn (operands[3], tmp1); set_unique_reg_note (insn, REG_EQUAL, mod); @@ -7369,10 +7387,13 @@ (ior:HI (ashift:HI (zero_extend:HI - (mod:QI (match_operand:HI 1 "register_operand" "0") + (mod:QI (subreg:QI + (match_operand:HI 1 "register_operand" "0") 0) (match_operand:QI 2 "nonimmediate_operand" "qm"))) (const_int 8)) - (zero_extend:HI (div:QI (match_dup 1) (match_dup 2))))) + (zero_extend:HI + (div:QI (subreg:QI (match_dup 1) 0) (match_dup 2))))) + (use (match_dup 1)) (clobber (reg:CC FLAGS_REG))] "TARGET_QIMODE_MATH" "idiv{b}\t%2" @@ -7384,10 +7405,13 @@ (ior:HI (ashift:HI (zero_extend:HI - (umod:QI (match_operand:HI 1 "register_operand" "0") + (umod:QI (subreg:QI + (match_operand:HI 1 "register_operand" "0") 0) (match_operand:QI 2 "nonimmediate_operand" "qm"))) (const_int 8)) - (zero_extend:HI (udiv:QI (match_dup 1) (match_dup 2))))) + (zero_extend:HI + (udiv:QI (subreg:QI (match_dup 1) 0) (match_dup 2))))) + (use (match_dup 1)) (clobber (reg:CC FLAGS_REG))] "TARGET_QIMODE_MATH" "div{b}\t%2" --- /dev/null 2010-06-16 10:11:06.602750711 -0700 +++ gcc/gcc/testsuite/gcc.dg/torture/pr44695.c 2010-06-29 11:49:12.186942482 -0700 @@ -0,0 +1,15 @@ +/* { dg-do compile } */ + +typedef unsigned char uint8_t; + +static uint8_t +safe_div_func_uint8_t_u_u (uint8_t ui1, uint8_t ui2) +{ + return ui2 ? ui2 : (ui1 / ui2); +} + +int +int81 (int x) +{ + return safe_div_func_uint8_t_u_u (1, 8 & x); +}