Re: PATCH: Properly check mode for x86 call/jmp address

[Uros Bizjak <ubizjak at gmail dot com>]

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: p.diff.txt
Description: Text document