Patchwork Fix i386 *_cconly shifts (PR target/46088)

login
register
mail settings
Submitter Jakub Jelinek
Date Nov. 11, 2010, 3:35 p.m.
Message ID <20101111153536.GG29412@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/70826/
State New
Headers show

Comments

Jakub Jelinek - Nov. 11, 2010, 3:35 p.m.
Hi!

On these two patterns ix86_binary_operator_ok can crash, because
the operands[0] argument (dst) is a
(clobber (match_scratch:SWI 0 "=<r>"))
Before reload recog may try to match the insn without clobbers and
just return in *pnum_clobbers how many clobbers need to be added, then
add_clobbers can be called to add them.  But that means
ix86_binary_operator_ok predicate can't look at operands[0], because
it might contain garbage.

As detailed in the PR, I think !MEM_P (operands[1]) is the only test
from ix86_binary_operator_ok that actually applies for these shift patterns,
bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Alternatively, e.g.
-   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
+   && (GET_CODE (PATTERN (insn)) == SET
+       ? !MEM_P (operands[1])
+       : ix86_binary_operator_ok (<CODE>, <MODE>mode, operands))"
or:
-   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
+   && ((!reload_completed && GET_CODE (PATTERN (insn)) == SET)
+       ? !MEM_P (operands[1])
+       : ix86_binary_operator_ok (<CODE>, <MODE>mode, operands))"
could probably work too, just haven't tested it (yet).

2010-11-11  Jakub Jelinek  <jakub@redhat.com>

	PR target/46088
	* config/i386/i386.md (*ashl<mode>3_cconly,
	*<shiftrt_insn><mode>3_cconly): Don't use ix86_binary_operator_ok,
	just ensure operands[1] is not MEM.

	* gcc.dg/pr46088.c: New test.


	Jakub
Richard Henderson - Nov. 11, 2010, 3:51 p.m.
On 11/11/2010 07:35 AM, Jakub Jelinek wrote:
> -   && ix86_binary_operator_ok (ASHIFT, <MODE>mode, operands)"
> +   /* Can't use ix86_binary_operator_ok here, as the scratch
> +      operand might be missing.  */
> +   && !MEM_P (operands[1])"

Better to change the predicate on op1 to register_operand,
and drop the test here inside the extra predicate.

Ok with that change.


r~

Patch

--- gcc/config/i386/i386.md.jj	2010-11-11 09:38:56.000000000 +0100
+++ gcc/config/i386/i386.md	2010-11-11 12:04:40.000000000 +0100
@@ -9724,7 +9724,9 @@  (define_insn "*ashl<mode>3_cconly"
 	&& (TARGET_SHIFT1
 	    || TARGET_DOUBLE_WITH_ADD)))
    && ix86_match_ccmode (insn, CCGOCmode)
-   && ix86_binary_operator_ok (ASHIFT, <MODE>mode, operands)"
+   /* Can't use ix86_binary_operator_ok here, as the scratch
+      operand might be missing.  */
+   && !MEM_P (operands[1])"
 {
   switch (get_attr_type (insn))
     {
@@ -10090,7 +10092,9 @@  (define_insn "*<shiftrt_insn><mode>3_cco
     || (operands[2] == const1_rtx
 	&& TARGET_SHIFT1))
    && ix86_match_ccmode (insn, CCGOCmode)
-   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
+   /* Can't use ix86_binary_operator_ok here, as the scratch
+      operand might be missing.  */
+   && !MEM_P (operands[1])"
 {
   if (operands[2] == const1_rtx
       && (TARGET_SHIFT1 || optimize_function_for_size_p (cfun)))
--- gcc/testsuite/gcc.dg/pr46088.c.jj	2010-11-11 12:09:08.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr46088.c	2010-11-11 12:08:49.000000000 +0100
@@ -0,0 +1,12 @@ 
+/* PR target/46088 */
+/* { dg-do compile } */
+/* { dg-options "-Os -fnon-call-exceptions -fpeel-loops" } */
+
+extern void bar (void);
+
+void
+foo (int i)
+{
+  if (i >> 3)
+    bar ();
+}