From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21507 invoked by alias); 11 Mar 2012 13:44:16 -0000 Received: (qmail 21497 invoked by uid 22791); 11 Mar 2012 13:44:15 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,TW_ZJ X-Spam-Check-By: sourceware.org Received: from mail-yx0-f175.google.com (HELO mail-yx0-f175.google.com) (209.85.213.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 11 Mar 2012 13:44:02 +0000 Received: by yenm3 with SMTP id m3so2006019yen.20 for ; Sun, 11 Mar 2012 06:44:01 -0700 (PDT) MIME-Version: 1.0 Received: by 10.236.138.110 with SMTP id z74mr9724974yhi.114.1331473441289; Sun, 11 Mar 2012 06:44:01 -0700 (PDT) Received: by 10.146.241.18 with HTTP; Sun, 11 Mar 2012 06:44:01 -0700 (PDT) In-Reply-To: References: <4F5669D2.8080805@redhat.com> Date: Sun, 11 Mar 2012 13:44:00 -0000 Message-ID: Subject: Re: PATCH: Properly check mode for x86 call/jmp address From: Uros Bizjak To: "H.J. Lu" Cc: Richard Henderson , gcc-patches@gcc.gnu.org, Jakub Jelinek Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-03/txt/msg00703.txt.bz2 On Sat, Mar 10, 2012 at 5:05 PM, H.J. Lu wrote: >>>>>>> =A0(define_insn "*call" >>>>>>> - =A0[(call (mem:QI (match_operand:P 0 "call_insn_operand" "zw")) >>>>>>> + =A0[(call (mem:QI (match_operand:C 0 "call_insn_operand" "zw")) >>>>>>> =A0 =A0 =A0 =A0(match_operand 1 "" ""))] >>>>>>> - =A0"!SIBLING_CALL_P (insn)" >>>>>>> + =A0"!SIBLING_CALL_P (insn) >>>>>>> + =A0 && (GET_CODE (operands[0]) =3D=3D SYMBOL_REF >>>>>>> + =A0 =A0 =A0 || GET_MODE (operands[0]) =3D=3D word_mode)" >>>>>> >>>>>> There are enough copies of this extra constraint that I wonder >>>>>> if it simply ought to be folded into call_insn_operand. >>>>>> >>>>>> Which would need to be changed to define_special_predicate, >>>>>> since you'd be doing your own mode checking. >>>>>> >>>>>> Probably similar changes to sibcall_insn_operand. >>>>> >>>>> Here is the updated patch. =A0I changed constant_call_address_operand >>>>> and call_register_no_elim_operand to use define_special_predicate. >>>>> OK for trunk? >>>> >>>> Please do not complicate matters that much. Just stick word_mode >>>> overrides for register operands in predicates.md, like in attached >>>> patch. These changed predicates now allow registers only in word_mode >>>> (and VOIDmode). >>>> >>>> You can now remove all new mode iterators and leave call patterns unto= uched. >>>> >>>> @@ -22940,14 +22940,18 @@ ix86_expand_call (rtx retval, rtx fnaddr, >>>> rtx callarg1, >>>> =A0 =A0 =A0 && GET_CODE (XEXP (fnaddr, 0)) =3D=3D SYMBOL_REF >>>> =A0 =A0 =A0 && !local_symbolic_operand (XEXP (fnaddr, 0), VOIDmode)) >>>> =A0 =A0 fnaddr =3D gen_rtx_MEM (QImode, construct_plt_address (XEXP (f= naddr, 0))); >>>> - =A0else if (sibcall >>>> - =A0 =A0 =A0 =A0 =A0? !sibcall_insn_operand (XEXP (fnaddr, 0), Pmode) >>>> - =A0 =A0 =A0 =A0 =A0: !call_insn_operand (XEXP (fnaddr, 0), Pmode)) >>>> + =A0else if (!(constant_call_address_operand (XEXP (fnaddr, 0), Pmode) >>>> + =A0 =A0 =A0 =A0 =A0 =A0|| call_register_no_elim_operand (XEXP (fnadd= r, 0), >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0word_mode) >>>> + =A0 =A0 =A0 =A0 =A0 =A0|| (!sibcall >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&& !TARGET_X32 >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&& memory_operand (XEXP (fnaddr, 0), = word_mode)))) >>>> =A0 =A0 { >>>> =A0 =A0 =A0 fnaddr =3D XEXP (fnaddr, 0); >>>> - =A0 =A0 =A0if (GET_MODE (fnaddr) !=3D Pmode) >>>> - =A0 =A0 =A0 fnaddr =3D convert_to_mode (Pmode, fnaddr, 1); >>>> - =A0 =A0 =A0fnaddr =3D gen_rtx_MEM (QImode, copy_to_mode_reg (Pmode, = fnaddr)); >>>> + =A0 =A0 =A0if (GET_MODE (fnaddr) !=3D word_mode) >>>> + =A0 =A0 =A0 fnaddr =3D convert_to_mode (word_mode, fnaddr, 1); >>>> + =A0 =A0 =A0fnaddr =3D gen_rtx_MEM (QImode, >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 copy_to_mode_reg= (word_mode, fnaddr)); >>>> =A0 =A0 } >>>> >>>> =A0 vec_len =3D 0; >>>> >>>> Please update the above part. It looks you don't even have to change >>>> condition with new predicates. Basically, you should only convert the >>>> address to word_mode instead of Pmode. >>>> >>>> + =A0if (TARGET_X32) >>>> + =A0 =A0operands[0] =3D convert_memory_address (word_mode, operands[0= ]); >>>> >>>> This addition to indirect_jump and tablejump should be the only >>>> change, needed in i386.md now. Please write the condition >>>> >>>> if (Pmode !=3D word_mode) >>>> >>>> for consistency. >>>> >>>> BTW: The attached patch was bootstrapped and regression tested on >>>> x86_64-pc-linux-gnu {,-m32}. >>>> >>>> Uros. >>> >>> It doesn't work: >>> >>> x.i:7:1: error: unrecognizable insn: >>> (call_insn/j 8 7 9 3 (call (mem:QI (reg:DI 62) [0 *foo.0_1 S1 A8]) >>> =A0 =A0 =A0 =A0(const_int 0 [0])) x.i:6 -1 >>> =A0 =A0 (nil) >>> =A0 =A0(nil)) >>> x.i:7:1: internal compiler error: in extract_insn, at recog.c:2123 >>> Please submit a full bug report, >>> with preprocessed source if appropriate. >>> See for instructions. >>> make: *** [x.s] Error 1 >>> >>> I will investigate it. >> >> For reference, attached is the complete patch that uses >> define_special_predicate. This patch works OK with the current >> mainline, with additional patch to i386.h, where >> >> Index: i386.h >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- i386.h =A0 =A0 =A0(revision 185079) >> +++ i386.h =A0 =A0 =A0(working copy) >> @@ -1744,7 +1744,7 @@ >> =A0/* Specify the machine mode that pointers have. >> =A0 =A0After generation of rtl, the compiler makes no further distinction >> =A0 =A0between pointers and any other objects of this machine mode. =A0*/ >> -#define Pmode (TARGET_64BIT ? DImode : SImode) >> +#define Pmode (TARGET_LP64 ? DImode : SImode) >> >> =A0/* A C expression whose value is zero if pointers that need to be ext= ended >> =A0 =A0from being `POINTER_SIZE' bits wide to `Pmode' are sign-extended = and > > I tested this patch and it passed all my x32 tests. Committed to mainline with following ChangeLog: 2012-03-11 H.J. Lu Uros Bizjak * config/i386/predicates.md (call_insn_operand): Allow constant_call_address_operand in Pmode only. (sibcall_insn_operand): Ditto. * config/i386/i386.md (*call): Use W mode iterator instead of P mode. (*call_vzeroupper): Ditto. (*sibcall): Ditto. (*sibcall_vzeroupper): Ditto. (*call_value): Ditto. (*call_value_vzeroupper): Ditto. (*sibcall_value): Ditto. (*sibcall_value_vzeroupper): Ditto. (*indirect_jump): Ditto. (*tablejump_1): Ditto. (indirect_jump): Convert memory address to word mode for TARGET_X32. (tablejump): Ditto. * config/i386/i386.c (ix86_expand_call): Convert indirect operands to word mode. Uros.