PATCH: Properly check mode for x86 call/jmp address
H.J. Lu
hjl.tools@gmail.com
Tue Mar 6 19:11:00 GMT 2012
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.
-------------- next part --------------
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")])
;; 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")])
More information about the Gcc-patches
mailing list