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.
Index: i386.c
===================================================================
--- i386.c (revision 185058)
+++ i386.c (working copy)
@@ -22952,9 +22952,9 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx call
: !call_insn_operand (XEXP (fnaddr, 0), Pmode))
{
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;
Index: i386.md
===================================================================
--- i386.md (revision 185073)
+++ i386.md (working copy)
@@ -11078,7 +11078,12 @@
(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"))]
@@ -11123,8 +11128,9 @@
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"
Index: predicates.md
===================================================================
--- predicates.md (revision 185073)
+++ 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.
@@ -565,22 +565,27 @@
(match_operand 0 "immediate_operand")))
;; Test for a valid operand for indirect branch.
-(define_predicate "indirect_branch_operand"
- (if_then_else (match_test "TARGET_X32")
- (match_operand 0 "register_operand")
- (match_operand 0 "nonimmediate_operand")))
+;; Allow register operands in word mode only.
+(define_special_predicate "indirect_branch_operand"
+ (ior (match_test "register_operand
+ (op, mode == VOIDmode ? mode : word_mode)")
+ (and (not (match_test "TARGET_X32"))
+ (match_operand 0 "memory_operand"))))
;; Test for a valid operand for a call instruction.
-(define_predicate "call_insn_operand"
+;; Allow register operands in word mode only.
+(define_special_predicate "call_insn_operand"
(ior (match_operand 0 "constant_call_address_operand")
- (match_operand 0 "call_register_no_elim_operand")
+ (match_test "call_register_no_elim_operand
+ (op, mode == VOIDmode ? mode : word_mode)")
(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"
+(define_special_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"
