On Wed, Mar 7, 2012 at 1:58 PM, Uros Bizjak <[email protected]> wrote:
> On Wed, Mar 7, 2012 at 5:03 PM, H.J. Lu <[email protected]> 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
>
> Uros.
I tested this patch and it passed all my x32 tests.
Thanks.
--
H.J.
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.
