From patchwork Wed Mar 7 09:28:08 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Uros Bizjak X-Patchwork-Id: 145173 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 07F38B6EF1 for ; Wed, 7 Mar 2012 20:28:31 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1331717312; h=Comment: DomainKey-Signature:Received:Received:Received:Received: MIME-Version:Received:Received:In-Reply-To:References:Date: Message-ID:Subject:From:To:Cc:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=/RGN0dZozMjB3++rrcpy4vNqR9g=; b=cA4p7GJz1GqW/3dVnICvZk+hMiZC+0gH6kC7j8e2rgX/Tjejb93TfhyizqzAXw Ceqtqu1Em8yXjY+xK6GTXSTITGqVZUx4TyWP3xOpIgXMw+D0E7bSMlIBY97v8i6T fKiycoG+W7Jo03QL0E6AoLKUwImBVe2LcMYnGMzvNQVSs= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:MIME-Version:Received:Received:In-Reply-To:References:Date:Message-ID:Subject:From:To:Cc:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=SviVc5cEGM7adXdNS5PKj1D+IDqW8PYrG5bluxZNb+BIu/l3GnfkHy8T0FkjFC tKqkboSP0RhbHrhYFURnq+4zHOYU6y242cPq5pNGqfzwHOWa8FlsWd9MWZvrlhOY 7Z10vtbVnmdEEmP8TB/xg9LeghtxvKq/jmUWcmpVpsMFA=; Received: (qmail 6443 invoked by alias); 7 Mar 2012 09:28:24 -0000 Received: (qmail 6435 invoked by uid 22791); 7 Mar 2012 09:28:23 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-tul01m020-f175.google.com (HELO mail-tul01m020-f175.google.com) (209.85.214.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 07 Mar 2012 09:28:09 +0000 Received: by obqv19 with SMTP id v19so7631470obq.20 for ; Wed, 07 Mar 2012 01:28:08 -0800 (PST) MIME-Version: 1.0 Received: by 10.60.7.7 with SMTP id f7mr529146oea.19.1331112488778; Wed, 07 Mar 2012 01:28:08 -0800 (PST) Received: by 10.182.225.67 with HTTP; Wed, 7 Mar 2012 01:28:08 -0800 (PST) In-Reply-To: References: <4F5669D2.8080805@redhat.com> Date: Wed, 7 Mar 2012 10:28:08 +0100 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 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 Tue, Mar 6, 2012 at 9:57 PM, H.J. Lu wrote: >>>  (define_insn "*call" >>> -  [(call (mem:QI (match_operand:P 0 "call_insn_operand" "zw")) >>> +  [(call (mem:QI (match_operand:C 0 "call_insn_operand" "zw")) >>>        (match_operand 1 "" ""))] >>> -  "!SIBLING_CALL_P (insn)" >>> +  "!SIBLING_CALL_P (insn) >>> +   && (GET_CODE (operands[0]) == SYMBOL_REF >>> +       || GET_MODE (operands[0]) == 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.  I 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 untouched. @@ -22940,14 +22940,18 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1, && GET_CODE (XEXP (fnaddr, 0)) == SYMBOL_REF && !local_symbolic_operand (XEXP (fnaddr, 0), VOIDmode)) fnaddr = gen_rtx_MEM (QImode, construct_plt_address (XEXP (fnaddr, 0))); - else if (sibcall - ? !sibcall_insn_operand (XEXP (fnaddr, 0), Pmode) - : !call_insn_operand (XEXP (fnaddr, 0), Pmode)) + else if (!(constant_call_address_operand (XEXP (fnaddr, 0), Pmode) + || call_register_no_elim_operand (XEXP (fnaddr, 0), + word_mode) + || (!sibcall + && !TARGET_X32 + && memory_operand (XEXP (fnaddr, 0), word_mode)))) { fnaddr = XEXP (fnaddr, 0); - if (GET_MODE (fnaddr) != Pmode) - fnaddr = convert_to_mode (Pmode, fnaddr, 1); - fnaddr = gen_rtx_MEM (QImode, copy_to_mode_reg (Pmode, fnaddr)); + if (GET_MODE (fnaddr) != word_mode) + fnaddr = convert_to_mode (word_mode, fnaddr, 1); + fnaddr = gen_rtx_MEM (QImode, + copy_to_mode_reg (word_mode, fnaddr)); } vec_len = 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. + if (TARGET_X32) + operands[0] = 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 != word_mode) for consistency. BTW: The attached patch was bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32}. Uros. Index: predicates.md =================================================================== --- predicates.md (revision 184992) +++ predicates.md (working copy) @@ -1,5 +1,5 @@ ;; Predicate definitions for IA-32 and x86-64. -;; Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 +;; Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 ;; Free Software Foundation, Inc. ;; ;; This file is part of GCC. @@ -557,22 +565,27 @@ (match_operand 0 "immediate_operand"))) ;; Test for a valid operand for indirect branch. +;; Allow register operands in word mode only. (define_predicate "indirect_branch_operand" - (if_then_else (match_test "TARGET_X32") - (match_operand 0 "register_operand") - (match_operand 0 "nonimmediate_operand"))) + (ior (match_test "register_operand + (op, mode == VOIDmode ? mode : word_mode)") + (and (match_test "Pmode == word_mode") + (match_operand 0 "memory_operand")))) ;; Test for a valid operand for a call instruction. +;; Allow register operands in word mode only. (define_predicate "call_insn_operand" (ior (match_operand 0 "constant_call_address_operand") - (match_operand 0 "call_register_no_elim_operand") - (and (not (match_test "TARGET_X32")) + (match_test "call_register_no_elim_operand + (op, mode == VOIDmode ? mode : word_mode)") + (and (match_test "Pmode == word_mode") (match_operand 0 "memory_operand")))) ;; Similarly, but for tail calls, in which we cannot allow memory references. (define_predicate "sibcall_insn_operand" (ior (match_operand 0 "constant_call_address_operand") - (match_operand 0 "register_no_elim_operand"))) + (match_test "register_no_elim_operand + (op, mode == VOIDmode ? mode : word_mode)"))) ;; Match exactly zero. (define_predicate "const0_operand"