Re: PATCH: Properly check mode for x86 call/jmp address
On Sat, Mar 10, 2012 at 5:05 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.
>>>
>>> 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 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
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.
Re: PATCH: Properly check mode for x86 call/jmp address
On Wed, Mar 7, 2012 at 1:58 PM, Uros Bizjak wrote:
> On Wed, Mar 7, 2012 at 5:03 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.
>>
>> 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 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
Re: PATCH: Properly check mode for x86 call/jmp address
On Wed, Mar 7, 2012 at 5:03 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.
>
> 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 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
===
Re: PATCH: Properly check mode for x86 call/jmp address
On Wed, Mar 7, 2012 at 1:28 AM, Uros Bizjak wrote:
> 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.
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 for instructions.
make: *** [x.s] Error 1
I will investigate it.
--
H.J.
Re: PATCH: Properly check mode for x86 call/jmp address
On Wed, Mar 7, 2012 at 11:07 AM, Uros Bizjak wrote: > Ah, I vaguely remember that indirect call/jmp is invalid on X32 for This should read "... indirect call/jmp FROM MEMORY is invalid on X32 ...". It looks I've had too much morning coffee already ;) Uros.
Re: PATCH: Properly check mode for x86 call/jmp address
On Wed, Mar 7, 2012 at 11:07 AM, Uros Bizjak wrote: > On Wed, Mar 7, 2012 at 10:28 AM, Uros Bizjak wrote: > >> + 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. > > Ah, I vaguely remember that indirect call/jmp is invalid on X32 for > some other reason. So, please leave the condition above as is and also > revert similar change in attached patch back to (not (match_test > "TARGET_X32")). Now with attached predicate.md patch. 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 (not (match_test "TARGET_X32")) + (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") + (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" (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"
Re: PATCH: Properly check mode for x86 call/jmp address
On Wed, Mar 7, 2012 at 10:28 AM, Uros Bizjak wrote: > + 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. Ah, I vaguely remember that indirect call/jmp is invalid on X32 for some other reason. So, please leave the condition above as is and also revert similar change in attached patch back to (not (match_test "TARGET_X32")). Uros.
Re: PATCH: Properly check mode for x86 call/jmp address
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"
Re: PATCH: Properly check mode for x86 call/jmp address
On Tue, Mar 6, 2012 at 11:47 AM, Richard Henderson wrote:
> On 03/06/12 11:10, 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.
>
>
> r~
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?
Thanks.
--
H.J
---
2012-03-06 H.J. Lu
* config/i386/i386.c (ix86_expand_call): Call
constant_call_address_operand with Pmode and call
call_register_no_elim_operand/memory_operand with word_mode.
Convert the address to word_mode instead of Pmode.
* config/i386/i386.md (W): New.
(C): Likewise.
(indirect_jump): Convert address to word_mode for x32.
(tablejump): Likewise.
(*indirect_jump): Replace :P with :W.
(*tablejump_1): Likewise.
(*call_vzeroupper): Replace :P with :C.
(*call): Likewise.
(*sibcall_vzeroupper): Likewise.
(*sibcall): Likewise.
(*call_value_vzeroupper): Likewise.
(*call_value): Likewise.
(*sibcall_value_vzeroupper): Likewise.
(*sibcall_value): Likewise.
* config/i386/predicates.md (constant_call_address_operand):
Defined with define_special_predicate. Return false if mode
isn't Pmode.
(call_register_no_elim_operand): Defined with
define_special_predicate. Return false if mode isn't word_mode.
2012-03-06 H.J. Lu
* config/i386/i386.c (ix86_expand_call): Call
constant_call_address_operand with Pmode and call
call_register_no_elim_operand/memory_operand with word_mode.
Convert the address to word_mode instead of Pmode.
* config/i386/i386.md (W): New.
(C): Likewise.
(indirect_jump): Convert address to word_mode for x32.
(tablejump): Likewise.
(*indirect_jump): Replace :P with :W.
(*tablejump_1): Likewise.
(*call_vzeroupper): Replace :P with :C.
(*call): Likewise.
(*sibcall_vzeroupper): Likewise.
(*sibcall): Likewise.
(*call_value_vzeroupper): Likewise.
(*call_value): Likewise.
(*sibcall_value_vzeroupper): Likewise.
(*sibcall_value): Likewise.
* config/i386/predicates.md (constant_call_address_operand):
Defined with define_special_predicate. Return false if mode
isn't Pmode.
(call_register_no_elim_operand): Defined with
define_special_predicate. Return false if mode isn't word_mode.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 973bbeb..7ee71fa 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -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;
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index bfbf5bf..801ffa2 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -896,6 +896,16 @@
;; ptr_mode sized quantities.
(define_mode_iterator PTR
[(SI "ptr_mode == SImode") (DI "ptr_mode == DImode")])
+
+;; This mode iterator allows :W to be used for patterns that operate on
+;; word_mode sized quantities.
+(define_mode_iterator W
+ [(SI "word_mode == SImode") (DI "word_mode == DImode")])
+
+;; This mode iterator allows :C to be used for patterns that operate on
+;; pointer
Re: PATCH: Properly check mode for x86 call/jmp address
On 03/06/12 11:10, 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. r~
Re: PATCH: Properly check mode for x86 call/jmp address
On Tue, Mar 6, 2012 at 8:10 PM, H.J. Lu wrote:
> On Tue, Mar 6, 2012 at 7:37 AM, H.J. Lu wrote:
>> On Mon, Mar 5, 2012 at 9:11 AM, H.J. Lu wrote:
>>> On Sun, Mar 4, 2012 at 11:47 PM, Uros Bizjak wrote:
On Mon, Mar 5, 2012 at 4:53 AM, H.J. Lu wrote:
> and compiler does generate the same output. i386.c also has
>
> xasm = "jmp\t%A0";
> xasm = "call\t%A0";
>
> for calls. There are no separate indirect call patterns. For x32,
> only indirect register calls have to be in DImode. The direct call
> should be in Pmode (SImode).
Direct call just expects label to some abolute address that is assumed
to fit in 32 bits (see constant_call_address_operand_p).
call and jmp insn expect word_mode operands, so please change
ix86_expand_call and call patterns in the same way as jump
instructions above.
> Since x86-64 hardware always zero-extends upper 32bits of 64bit
> registers when loading its lower 32bits, it is safe and easier to just
> to output 64bit registers for %A than zero-extend it by hand for all
> jump/call patterns.
No, the instruction expects word_mode operands, so we have to extend
values to expected mode. I don't think that patching at insn output
time is acceptable.
>>>
>>> You are right. I found a testcase to show problem:
>>>
>>> struct foo
>>> {
>>> void (*f) (void);
>>> int i;
>>> };
>>>
>>> void
>>> __attribute__ ((noinline))
>>> bar (struct foo x)
>>> {
>>> x.f ();
>>> }
>>>
>>> "x" is passed in RDI and the uppper 32bits of RDI is "int i".
>>>
>>
>
> Hi,
>
> This is what I come up with. This patch enforces word_mode for
> non-SYMBOL_REF call/jmp address while allowing Pmode for
> SYMBOL_REF call/jmp address. I added W for word_mode
> used on indirect branches and added C for Pmode/word_mode
> used on calls. For calls, I added check for SYMBOL_REF address
> or address in word_mode since non-SYMBOL_REF address must
> be in word_mode. Tested on Linux/x86-64.
>
> OK for trunk?
(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:C 1 "sibcall_insn_operand" "Uz"))
(match_operand 2 "" "")))
(unspec [(match_operand 3 "const_int_operand" "")]
UNSPEC_CALL_NEEDS_VZEROUPPER)]
- "TARGET_VZEROUPPER && SIBLING_CALL_P (insn)"
+ "TARGET_VZEROUPPER
+ && SIBLING_CALL_P (insn)
+ && ((GET_CODE (operands[1]) == SYMBOL_REF
+ && GET_MODE (operands[1]) == Pmode)
+ || (GET_CODE (operands[1]) != SYMBOL_REF
+ && GET_MODE (operands[1]) == word_mode))"
"#"
"&& reload_completed"
[(const_int 0)]
Why does this patterh have different insn constraitn than all others?
BTW - please do not split existing "TARGET_VZEROUPPER &&
SIBLING_CALL_P (insn)" lines ...
Uros.
