* PATCH: Properly check mode for x86 call/jmp address
@ 2012-03-06 19:11 H.J. Lu
2012-03-06 19:42 ` Uros Bizjak
2012-03-06 19:47 ` Richard Henderson
0 siblings, 2 replies; 12+ messages in thread
From: H.J. Lu @ 2012-03-06 19:11 UTC (permalink / raw)
To: Uros Bizjak; +Cc: gcc-patches, Richard Henderson, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 2881 bytes --]
On Tue, Mar 6, 2012 at 7:37 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Mar 5, 2012 at 9:11 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Sun, Mar 4, 2012 at 11:47 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Mon, Mar 5, 2012 at 4:53 AM, H.J. Lu <hjl.tools@gmail.com> 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?
Thanks.
--
H.J.
---
2012-03-06 H.J. Lu <hongjiu.lu@intel.com>
* 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. Check address is
SYMBOL_REF or in word_mode.
(*call): Likewise.
(*sibcall_vzeroupper): Likewise.
(*sibcall): Likewise.
(*call_value_vzeroupper): Likewise.
(*call_value): Likewise.
(*sibcall_value_vzeroupper): Likewise.
(*sibcall_value): Likewise.
[-- Attachment #2: gcc-x32-branch-1.patch --]
[-- Type: text/plain, Size: 8169 bytes --]
2012-03-06 H.J. Lu <hongjiu.lu@intel.com>
* 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. Check address is
SYMBOL_REF or in word_mode.
(*call): Likewise.
(*sibcall_vzeroupper): Likewise.
(*sibcall): Likewise.
(*call_value_vzeroupper): Likewise.
(*call_value): Likewise.
(*sibcall_value_vzeroupper): Likewise.
(*sibcall_value): Likewise.
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..bb3647a 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-sized and word_mode sized quantities.
+(define_mode_iterator C
+ [(SI "Pmode == SImode") (DI "word_mode == DImode")])
\f
;; Scheduling descriptions
@@ -11076,10 +11086,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")
@@ -11121,12 +11136,12 @@
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"
@@ -11213,11 +11228,14 @@
})
(define_insn_and_split "*call_vzeroupper"
- [(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 "" ""))
(unspec [(match_operand 2 "const_int_operand" "")]
UNSPEC_CALL_NEEDS_VZEROUPPER)]
- "TARGET_VZEROUPPER && !SIBLING_CALL_P (insn)"
+ "TARGET_VZEROUPPER
+ && !SIBLING_CALL_P (insn)
+ && (GET_CODE (operands[0]) == SYMBOL_REF
+ || GET_MODE (operands[0]) == word_mode)"
"#"
"&& reload_completed"
[(const_int 0)]
@@ -11225,9 +11243,11 @@
[(set_attr "type" "call")])
(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)"
"* return ix86_output_call_insn (insn, operands[0]);"
[(set_attr "type" "call")])
@@ -11277,11 +11297,14 @@
[(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:C 0 "sibcall_insn_operand" "Uz"))
(match_operand 1 "" ""))
(unspec [(match_operand 2 "const_int_operand" "")]
UNSPEC_CALL_NEEDS_VZEROUPPER)]
- "TARGET_VZEROUPPER && SIBLING_CALL_P (insn)"
+ "TARGET_VZEROUPPER
+ && SIBLING_CALL_P (insn)
+ && (GET_CODE (operands[0]) == SYMBOL_REF
+ || GET_MODE (operands[0]) == word_mode)"
"#"
"&& reload_completed"
[(const_int 0)]
@@ -11289,9 +11312,11 @@
[(set_attr "type" "call")])
(define_insn "*sibcall"
- [(call (mem:QI (match_operand:P 0 "sibcall_insn_operand" "Uz"))
+ [(call (mem:QI (match_operand:C 0 "sibcall_insn_operand" "Uz"))
(match_operand 1 "" ""))]
- "SIBLING_CALL_P (insn)"
+ "SIBLING_CALL_P (insn)
+ && (GET_CODE (operands[0]) == SYMBOL_REF
+ || GET_MODE (operands[0]) == word_mode)"
"* return ix86_output_call_insn (insn, operands[0]);"
[(set_attr "type" "call")])
@@ -11386,11 +11411,14 @@
(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:C 1 "call_insn_operand" "<c>zw"))
(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]) == word_mode)"
"#"
"&& reload_completed"
[(const_int 0)]
@@ -11399,19 +11427,26 @@
(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:C 1 "call_insn_operand" "<c>zw"))
(match_operand 2 "" "")))]
- "!SIBLING_CALL_P (insn)"
+ "!SIBLING_CALL_P (insn)
+ && (GET_CODE (operands[1]) == SYMBOL_REF
+ || GET_MODE (operands[1]) == word_mode)"
"* return ix86_output_call_insn (insn, operands[1]);"
[(set_attr "type" "callv")])
(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)]
@@ -11420,9 +11455,11 @@
(define_insn "*sibcall_value"
[(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 "" "")))]
- "SIBLING_CALL_P (insn)"
+ "SIBLING_CALL_P (insn)
+ && (GET_CODE (operands[1]) == SYMBOL_REF
+ || GET_MODE (operands[1]) == word_mode)"
"* return ix86_output_call_insn (insn, operands[1]);"
[(set_attr "type" "callv")])
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: Properly check mode for x86 call/jmp address
2012-03-06 19:11 PATCH: Properly check mode for x86 call/jmp address H.J. Lu
@ 2012-03-06 19:42 ` Uros Bizjak
2012-03-06 19:47 ` Richard Henderson
1 sibling, 0 replies; 12+ messages in thread
From: Uros Bizjak @ 2012-03-06 19:42 UTC (permalink / raw)
To: H.J. Lu; +Cc: gcc-patches, Richard Henderson, Jakub Jelinek
On Tue, Mar 6, 2012 at 8:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Mar 6, 2012 at 7:37 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Mar 5, 2012 at 9:11 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Sun, Mar 4, 2012 at 11:47 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>> On Mon, Mar 5, 2012 at 4:53 AM, H.J. Lu <hjl.tools@gmail.com> 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: Properly check mode for x86 call/jmp address
2012-03-06 19:11 PATCH: Properly check mode for x86 call/jmp address H.J. Lu
2012-03-06 19:42 ` Uros Bizjak
@ 2012-03-06 19:47 ` Richard Henderson
2012-03-06 20:57 ` H.J. Lu
1 sibling, 1 reply; 12+ messages in thread
From: Richard Henderson @ 2012-03-06 19:47 UTC (permalink / raw)
To: H.J. Lu; +Cc: Uros Bizjak, gcc-patches, Jakub Jelinek
On 03/06/12 11:10, H.J. Lu 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.
r~
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: Properly check mode for x86 call/jmp address
2012-03-06 19:47 ` Richard Henderson
@ 2012-03-06 20:57 ` H.J. Lu
2012-03-07 9:28 ` Uros Bizjak
0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2012-03-06 20:57 UTC (permalink / raw)
To: Richard Henderson; +Cc: Uros Bizjak, gcc-patches, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]
On Tue, Mar 6, 2012 at 11:47 AM, Richard Henderson <rth@redhat.com> wrote:
> On 03/06/12 11:10, H.J. Lu 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.
>
>
> 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 <hongjiu.lu@intel.com>
* 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.
[-- Attachment #2: gcc-x32-branch-2.patch --]
[-- Type: text/plain, Size: 7779 bytes --]
2012-03-06 H.J. Lu <hongjiu.lu@intel.com>
* 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-sized and word_mode sized quantities.
+(define_mode_iterator C
+ [(SI "Pmode == SImode") (DI "word_mode == DImode")])
\f
;; Scheduling descriptions
@@ -11076,10 +11086,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")
@@ -11121,12 +11136,12 @@
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"
@@ -11213,7 +11228,7 @@
})
(define_insn_and_split "*call_vzeroupper"
- [(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 "" ""))
(unspec [(match_operand 2 "const_int_operand" "")]
UNSPEC_CALL_NEEDS_VZEROUPPER)]
@@ -11225,7 +11240,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:C 0 "call_insn_operand" "<c>zw"))
(match_operand 1 "" ""))]
"!SIBLING_CALL_P (insn)"
"* return ix86_output_call_insn (insn, operands[0]);"
@@ -11277,7 +11292,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:C 0 "sibcall_insn_operand" "Uz"))
(match_operand 1 "" ""))
(unspec [(match_operand 2 "const_int_operand" "")]
UNSPEC_CALL_NEEDS_VZEROUPPER)]
@@ -11289,7 +11304,7 @@
[(set_attr "type" "call")])
(define_insn "*sibcall"
- [(call (mem:QI (match_operand:P 0 "sibcall_insn_operand" "Uz"))
+ [(call (mem:QI (match_operand:C 0 "sibcall_insn_operand" "Uz"))
(match_operand 1 "" ""))]
"SIBLING_CALL_P (insn)"
"* return ix86_output_call_insn (insn, operands[0]);"
@@ -11386,7 +11401,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:C 1 "call_insn_operand" "<c>zw"))
(match_operand 2 "" "")))
(unspec [(match_operand 3 "const_int_operand" "")]
UNSPEC_CALL_NEEDS_VZEROUPPER)]
@@ -11399,7 +11414,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:C 1 "call_insn_operand" "<c>zw"))
(match_operand 2 "" "")))]
"!SIBLING_CALL_P (insn)"
"* return ix86_output_call_insn (insn, operands[1]);"
@@ -11407,7 +11422,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:C 1 "sibcall_insn_operand" "Uz"))
(match_operand 2 "" "")))
(unspec [(match_operand 3 "const_int_operand" "")]
UNSPEC_CALL_NEEDS_VZEROUPPER)]
@@ -11420,7 +11435,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:C 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 32f73da..4853bff 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -492,9 +492,11 @@
(match_test "op == ix86_tls_module_base ()")))
;; Test for a pc-relative call operand
-(define_predicate "constant_call_address_operand"
+(define_special_predicate "constant_call_address_operand"
(match_code "symbol_ref")
{
+ if (mode != Pmode)
+ return false;
if (ix86_cmodel == CM_LARGE || ix86_cmodel == CM_LARGE_PIC)
return false;
if (TARGET_DLLIMPORT_DECL_ATTRIBUTES && SYMBOL_REF_DLLIMPORT_P (op))
@@ -506,9 +508,12 @@
;; is used as a call operand, so they will execute return address as a code.
;; See Pentium Pro errata 70, Pentium 2 errata A33 and Pentium 3 errata E17.
-(define_predicate "call_register_no_elim_operand"
+(define_special_predicate "call_register_no_elim_operand"
(match_operand 0 "register_operand")
{
+ if (mode != word_mode)
+ return false;
+
if (GET_CODE (op) == SUBREG)
op = SUBREG_REG (op);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: Properly check mode for x86 call/jmp address
2012-03-06 20:57 ` H.J. Lu
@ 2012-03-07 9:28 ` Uros Bizjak
2012-03-07 10:07 ` Uros Bizjak
2012-03-07 16:04 ` H.J. Lu
0 siblings, 2 replies; 12+ messages in thread
From: Uros Bizjak @ 2012-03-07 9:28 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Henderson, gcc-patches, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 2854 bytes --]
On Tue, Mar 6, 2012 at 9:57 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.
[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 1849 bytes --]
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"
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: Properly check mode for x86 call/jmp address
2012-03-07 9:28 ` Uros Bizjak
@ 2012-03-07 10:07 ` Uros Bizjak
2012-03-07 10:13 ` Uros Bizjak
2012-03-07 10:22 ` Uros Bizjak
2012-03-07 16:04 ` H.J. Lu
1 sibling, 2 replies; 12+ messages in thread
From: Uros Bizjak @ 2012-03-07 10:07 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Henderson, gcc-patches, Jakub Jelinek
On Wed, Mar 7, 2012 at 10:28 AM, Uros Bizjak <ubizjak@gmail.com> 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: Properly check mode for x86 call/jmp address
2012-03-07 10:07 ` Uros Bizjak
@ 2012-03-07 10:13 ` Uros Bizjak
2012-03-07 10:22 ` Uros Bizjak
1 sibling, 0 replies; 12+ messages in thread
From: Uros Bizjak @ 2012-03-07 10:13 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Henderson, gcc-patches, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 722 bytes --]
On Wed, Mar 7, 2012 at 11:07 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Mar 7, 2012 at 10:28 AM, Uros Bizjak <ubizjak@gmail.com> 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.
[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 1800 bytes --]
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"
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: Properly check mode for x86 call/jmp address
2012-03-07 10:07 ` Uros Bizjak
2012-03-07 10:13 ` Uros Bizjak
@ 2012-03-07 10:22 ` Uros Bizjak
1 sibling, 0 replies; 12+ messages in thread
From: Uros Bizjak @ 2012-03-07 10:22 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Henderson, gcc-patches, Jakub Jelinek
On Wed, Mar 7, 2012 at 11:07 AM, Uros Bizjak <ubizjak@gmail.com> 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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: Properly check mode for x86 call/jmp address
2012-03-07 9:28 ` Uros Bizjak
2012-03-07 10:07 ` Uros Bizjak
@ 2012-03-07 16:04 ` H.J. Lu
2012-03-07 21:58 ` Uros Bizjak
1 sibling, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2012-03-07 16:04 UTC (permalink / raw)
To: Uros Bizjak; +Cc: Richard Henderson, gcc-patches, Jakub Jelinek
On Wed, Mar 7, 2012 at 1:28 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Tue, Mar 6, 2012 at 9:57 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.
--
H.J.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: Properly check mode for x86 call/jmp address
2012-03-07 16:04 ` H.J. Lu
@ 2012-03-07 21:58 ` Uros Bizjak
2012-03-10 16:05 ` H.J. Lu
0 siblings, 1 reply; 12+ messages in thread
From: Uros Bizjak @ 2012-03-07 21:58 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Henderson, gcc-patches, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 4415 bytes --]
On Wed, Mar 7, 2012 at 5:03 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
Uros.
[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 3507 bytes --]
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"
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: Properly check mode for x86 call/jmp address
2012-03-07 21:58 ` Uros Bizjak
@ 2012-03-10 16:05 ` H.J. Lu
2012-03-11 13:44 ` Uros Bizjak
0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2012-03-10 16:05 UTC (permalink / raw)
To: Uros Bizjak; +Cc: Richard Henderson, gcc-patches, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 4690 bytes --]
On Wed, Mar 7, 2012 at 1:58 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Mar 7, 2012 at 5:03 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
>
> Uros.
I tested this patch and it passed all my x32 tests.
Thanks.
--
H.J.
[-- Attachment #2: gcc-x32-branch-3.patch --]
[-- Type: text/plain, Size: 6418 bytes --]
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: PATCH: Properly check mode for x86 call/jmp address
2012-03-10 16:05 ` H.J. Lu
@ 2012-03-11 13:44 ` Uros Bizjak
0 siblings, 0 replies; 12+ messages in thread
From: Uros Bizjak @ 2012-03-11 13:44 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Henderson, gcc-patches, Jakub Jelinek
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-03-11 13:44 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-06 19:11 PATCH: Properly check mode for x86 call/jmp address H.J. Lu
2012-03-06 19:42 ` Uros Bizjak
2012-03-06 19:47 ` Richard Henderson
2012-03-06 20:57 ` H.J. Lu
2012-03-07 9:28 ` Uros Bizjak
2012-03-07 10:07 ` Uros Bizjak
2012-03-07 10:13 ` Uros Bizjak
2012-03-07 10:22 ` Uros Bizjak
2012-03-07 16:04 ` H.J. Lu
2012-03-07 21:58 ` Uros Bizjak
2012-03-10 16:05 ` H.J. Lu
2012-03-11 13:44 ` Uros Bizjak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).