| Message ID | CAMe9rOqC_A+E1J68DZLfdMGrtGPm8FQOgs3+G==Cb7NTc7y0bg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Mar 10, 2012 at 5:05 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>>>>> (define_insn "*call" >>>>>>> - [(call (mem:QI (match_operand:P 0 "call_insn_operand" "<c>zw")) >>>>>>> + [(call (mem:QI (match_operand:C 0 "call_insn_operand" "<c>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. >>> >>> 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]) >>> (const_int 0 [0])) x.i:6 -1 >>> (nil) >>> (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 <http://gcc.gnu.org/bugs.html> 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 >> =================================================================== >> --- i386.h (revision 185079) >> +++ i386.h (working copy) >> @@ -1744,7 +1744,7 @@ >> /* Specify the machine mode that pointers have. >> After generation of rtl, the compiler makes no further distinction >> between pointers and any other objects of this machine mode. */ >> -#define Pmode (TARGET_64BIT ? DImode : SImode) >> +#define Pmode (TARGET_LP64 ? DImode : SImode) >> >> /* A C expression whose value is zero if pointers that need to be extended >> from 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 <hongjiu.lu@intel.com> Uros Bizjak <ubizjak@gmail.com> * 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.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index c2cad5a..33ef330 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -23032,13 +23031,13 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1, && !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)) + ? !sibcall_insn_operand (XEXP (fnaddr, 0), word_mode) + : !call_insn_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; diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index e3b5bd2..3994b1e 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -11091,10 +11091,15 @@ (set_attr "modrm" "0")]) (define_expand "indirect_jump" - [(set (pc) (match_operand 0 "indirect_branch_operand" ""))]) + [(set (pc) (match_operand 0 "indirect_branch_operand" ""))] + "" +{ + if (TARGET_X32) + operands[0] = convert_memory_address (word_mode, operands[0]); +}) (define_insn "*indirect_jump" - [(set (pc) (match_operand:P 0 "indirect_branch_operand" "rw"))] + [(set (pc) (match_operand:W 0 "indirect_branch_operand" "rw"))] "" "jmp\t%A0" [(set_attr "type" "ibr") @@ -11136,12 +11141,13 @@ operands[0] = expand_simple_binop (Pmode, code, op0, op1, NULL_RTX, 0, OPTAB_DIRECT); } - else if (TARGET_X32) - operands[0] = convert_memory_address (Pmode, operands[0]); + + if (TARGET_X32) + operands[0] = convert_memory_address (word_mode, operands[0]); }) (define_insn "*tablejump_1" - [(set (pc) (match_operand:P 0 "indirect_branch_operand" "rw")) + [(set (pc) (match_operand:W 0 "indirect_branch_operand" "rw")) (use (label_ref (match_operand 1 "" "")))] "" "jmp\t%A0" @@ -11228,7 +11234,7 @@ }) (define_insn_and_split "*call_vzeroupper" - [(call (mem:QI (match_operand:P 0 "call_insn_operand" "<c>zw")) + [(call (mem:QI (match_operand:W 0 "call_insn_operand" "<c>zw")) (match_operand 1 "" "")) (unspec [(match_operand 2 "const_int_operand" "")] UNSPEC_CALL_NEEDS_VZEROUPPER)] @@ -11240,7 +11246,7 @@ [(set_attr "type" "call")]) (define_insn "*call" - [(call (mem:QI (match_operand:P 0 "call_insn_operand" "<c>zw")) + [(call (mem:QI (match_operand:W 0 "call_insn_operand" "<c>zw")) (match_operand 1 "" ""))] "!SIBLING_CALL_P (insn)" "* return ix86_output_call_insn (insn, operands[0]);" @@ -11292,7 +11298,7 @@ [(set_attr "type" "call")]) (define_insn_and_split "*sibcall_vzeroupper" - [(call (mem:QI (match_operand:P 0 "sibcall_insn_operand" "Uz")) + [(call (mem:QI (match_operand:W 0 "sibcall_insn_operand" "Uz")) (match_operand 1 "" "")) (unspec [(match_operand 2 "const_int_operand" "")] UNSPEC_CALL_NEEDS_VZEROUPPER)] @@ -11304,7 +11310,7 @@ [(set_attr "type" "call")]) (define_insn "*sibcall" - [(call (mem:QI (match_operand:P 0 "sibcall_insn_operand" "Uz")) + [(call (mem:QI (match_operand:W 0 "sibcall_insn_operand" "Uz")) (match_operand 1 "" ""))] "SIBLING_CALL_P (insn)" "* return ix86_output_call_insn (insn, operands[0]);" @@ -11401,7 +11407,7 @@ (define_insn_and_split "*call_value_vzeroupper" [(set (match_operand 0 "" "") - (call (mem:QI (match_operand:P 1 "call_insn_operand" "<c>zw")) + (call (mem:QI (match_operand:W 1 "call_insn_operand" "<c>zw")) (match_operand 2 "" ""))) (unspec [(match_operand 3 "const_int_operand" "")] UNSPEC_CALL_NEEDS_VZEROUPPER)] @@ -11414,7 +11420,7 @@ (define_insn "*call_value" [(set (match_operand 0 "" "") - (call (mem:QI (match_operand:P 1 "call_insn_operand" "<c>zw")) + (call (mem:QI (match_operand:W 1 "call_insn_operand" "<c>zw")) (match_operand 2 "" "")))] "!SIBLING_CALL_P (insn)" "* return ix86_output_call_insn (insn, operands[1]);" @@ -11422,7 +11428,7 @@ (define_insn_and_split "*sibcall_value_vzeroupper" [(set (match_operand 0 "" "") - (call (mem:QI (match_operand:P 1 "sibcall_insn_operand" "Uz")) + (call (mem:QI (match_operand:W 1 "sibcall_insn_operand" "Uz")) (match_operand 2 "" ""))) (unspec [(match_operand 3 "const_int_operand" "")] UNSPEC_CALL_NEEDS_VZEROUPPER)] @@ -11435,7 +11441,7 @@ (define_insn "*sibcall_value" [(set (match_operand 0 "" "") - (call (mem:QI (match_operand:P 1 "sibcall_insn_operand" "Uz")) + (call (mem:QI (match_operand:W 1 "sibcall_insn_operand" "Uz")) (match_operand 2 "" "")))] "SIBLING_CALL_P (insn)" "* return ix86_output_call_insn (insn, operands[1]);" diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md index dad8bf3..7e0e1ef 100644 --- a/gcc/config/i386/predicates.md +++ b/gcc/config/i386/predicates.md @@ -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. @@ -571,15 +571,18 @@ (match_operand 0 "memory_operand")))) ;; Test for a valid operand for a call instruction. -(define_predicate "call_insn_operand" - (ior (match_operand 0 "constant_call_address_operand") +;; Allow constant call address operands in Pmode only. +(define_special_predicate "call_insn_operand" + (ior (match_test "constant_call_address_operand + (op, mode == VOIDmode ? mode : Pmode)") (match_operand 0 "call_register_no_elim_operand") (and (not (match_test "TARGET_X32")) (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") +(define_special_predicate "sibcall_insn_operand" + (ior (match_test "constant_call_address_operand + (op, mode == VOIDmode ? mode : Pmode)") (match_operand 0 "register_no_elim_operand"))) ;; Match exactly zero.