public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919)
@ 2016-10-27 19:34 Jakub Jelinek
  2016-10-28  7:15 ` Eric Botcazou
  2016-10-28  7:25 ` Richard Biener
  0 siblings, 2 replies; 9+ messages in thread
From: Jakub Jelinek @ 2016-10-27 19:34 UTC (permalink / raw)
  To: Eric Botcazou, Richard Biener; +Cc: gcc-patches

Hi!

The following testcase ICEs on x86_64-linux with -O1, the problem is
that we expand assignment from COMPONENT_REF of MEM_REF into a V4SImode
SSA_NAME.  The MEM_REF has non-addressable DCmode var inside of it, and
type of a struct containing a single V4SImode element.

The bug seems to be that if the op0 (i.e. get_inner_reference expanded)
is a CONCAT and we want a reference that covers all bits of the CONCAT,
we just short path it and return it immediately, rather than trying
to convert it to the requested mode.

I've bootstrapped/regtested on x86_64-linux and i686-linux the following
patch, which takes the short path only if we want a complex mode.
The only place it makes a difference in both bootstraps/regtests was this
new testcase.

Though, perhaps COMPLEX_MODE_P (mode1) is also wrong, if mode1 isn't
GET_MODE (op0), then we still will return something with unexpected mode
(e.g. DCmode vs. CDImode); I wonder if for such mismatches we shouldn't
just force_reg (convert_modes ()) each CONCAT operand separately and
create a new CONCAT.  Do we have a guarantee that COMPLEX_MODE_P (GET_MODE (op0))
if op0 is CONCAT?

2016-10-27  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/77919
	* expr.c (expand_expr_real_1) <normal_inner_ref>: Force CONCAT into
	MEM if mode1 is not a complex mode.

	* g++.dg/torture/pr77919.C: New test.

--- gcc/expr.c.jj	2016-10-27 20:50:22.699586175 +0200
+++ gcc/expr.c	2016-10-27 21:15:30.146309091 +0200
@@ -10421,7 +10421,8 @@ expand_expr_real_1 (tree exp, rtx target
 	if (GET_CODE (op0) == CONCAT && !must_force_mem)
 	  {
 	    if (bitpos == 0
-		&& bitsize == GET_MODE_BITSIZE (GET_MODE (op0)))
+		&& bitsize == GET_MODE_BITSIZE (GET_MODE (op0))
+		&& COMPLEX_MODE_P (mode1))
 	      {
 		if (reversep)
 		  op0 = flip_storage_order (GET_MODE (op0), op0);
--- gcc/testsuite/g++.dg/torture/pr77919.C.jj	2016-10-27 21:15:19.883440139 +0200
+++ gcc/testsuite/g++.dg/torture/pr77919.C	2016-10-27 21:16:01.694906242 +0200
@@ -0,0 +1,11 @@
+// PR rtl-optimization/77919
+// { dg-do compile }
+// { dg-additional-options "-Wno-psabi" }
+
+struct A { A (double) {} _Complex double i; };
+typedef int __attribute__ ((vector_size (16))) B;
+typedef struct { B b; } C;
+struct D { D (const B &x) : b (x) {} B b; };
+static inline B foo (const double *x) { C *a; a = (C *) x; return a->b; }
+static inline D baz (const A &x) { return foo ((double *) &x); }
+D b = baz (0);

	Jakub

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919)
  2016-10-27 19:34 [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919) Jakub Jelinek
@ 2016-10-28  7:15 ` Eric Botcazou
  2016-10-28  7:25 ` Richard Biener
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Botcazou @ 2016-10-28  7:15 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Richard Biener

> Though, perhaps COMPLEX_MODE_P (mode1) is also wrong, if mode1 isn't
> GET_MODE (op0), then we still will return something with unexpected mode
> (e.g. DCmode vs. CDImode); I wonder if for such mismatches we shouldn't
> just force_reg (convert_modes ()) each CONCAT operand separately and
> create a new CONCAT.  Do we have a guarantee that COMPLEX_MODE_P (GET_MODE
> (op0)) if op0 is CONCAT?

In practice, at this point of the pipeline, I'd think so.

> 2016-10-27  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR rtl-optimization/77919
> 	* expr.c (expand_expr_real_1) <normal_inner_ref>: Force CONCAT into
> 	MEM if mode1 is not a complex mode.
> 
> 	* g++.dg/torture/pr77919.C: New test.

I think that's OK.

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919)
  2016-10-27 19:34 [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919) Jakub Jelinek
  2016-10-28  7:15 ` Eric Botcazou
@ 2016-10-28  7:25 ` Richard Biener
  2016-10-28  7:32   ` Jeff Law
  1 sibling, 1 reply; 9+ messages in thread
From: Richard Biener @ 2016-10-28  7:25 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Eric Botcazou, gcc-patches

On Thu, 27 Oct 2016, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase ICEs on x86_64-linux with -O1, the problem is
> that we expand assignment from COMPONENT_REF of MEM_REF into a V4SImode
> SSA_NAME.  The MEM_REF has non-addressable DCmode var inside of it, and
> type of a struct containing a single V4SImode element.
> 
> The bug seems to be that if the op0 (i.e. get_inner_reference expanded)
> is a CONCAT and we want a reference that covers all bits of the CONCAT,
> we just short path it and return it immediately, rather than trying
> to convert it to the requested mode.
> 
> I've bootstrapped/regtested on x86_64-linux and i686-linux the following
> patch, which takes the short path only if we want a complex mode.
> The only place it makes a difference in both bootstraps/regtests was this
> new testcase.
> 
> Though, perhaps COMPLEX_MODE_P (mode1) is also wrong, if mode1 isn't
> GET_MODE (op0), then we still will return something with unexpected mode
> (e.g. DCmode vs. CDImode); I wonder if for such mismatches we shouldn't
> just force_reg (convert_modes ()) each CONCAT operand separately and
> create a new CONCAT.  Do we have a guarantee that COMPLEX_MODE_P (GET_MODE (op0))
> if op0 is CONCAT?

I think so.  I'll leave the rest to people more familiar with RTL 
expansion -- generally I thought the callers of expand() have to deal
with expansions that return a different mode?

Richard.

> 2016-10-27  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR rtl-optimization/77919
> 	* expr.c (expand_expr_real_1) <normal_inner_ref>: Force CONCAT into
> 	MEM if mode1 is not a complex mode.
> 
> 	* g++.dg/torture/pr77919.C: New test.
> 
> --- gcc/expr.c.jj	2016-10-27 20:50:22.699586175 +0200
> +++ gcc/expr.c	2016-10-27 21:15:30.146309091 +0200
> @@ -10421,7 +10421,8 @@ expand_expr_real_1 (tree exp, rtx target
>  	if (GET_CODE (op0) == CONCAT && !must_force_mem)
>  	  {
>  	    if (bitpos == 0
> -		&& bitsize == GET_MODE_BITSIZE (GET_MODE (op0)))
> +		&& bitsize == GET_MODE_BITSIZE (GET_MODE (op0))
> +		&& COMPLEX_MODE_P (mode1))
>  	      {
>  		if (reversep)
>  		  op0 = flip_storage_order (GET_MODE (op0), op0);
> --- gcc/testsuite/g++.dg/torture/pr77919.C.jj	2016-10-27 21:15:19.883440139 +0200
> +++ gcc/testsuite/g++.dg/torture/pr77919.C	2016-10-27 21:16:01.694906242 +0200
> @@ -0,0 +1,11 @@
> +// PR rtl-optimization/77919
> +// { dg-do compile }
> +// { dg-additional-options "-Wno-psabi" }
> +
> +struct A { A (double) {} _Complex double i; };
> +typedef int __attribute__ ((vector_size (16))) B;
> +typedef struct { B b; } C;
> +struct D { D (const B &x) : b (x) {} B b; };
> +static inline B foo (const double *x) { C *a; a = (C *) x; return a->b; }
> +static inline D baz (const A &x) { return foo ((double *) &x); }
> +D b = baz (0);
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919)
  2016-10-28  7:25 ` Richard Biener
@ 2016-10-28  7:32   ` Jeff Law
  2016-10-28  8:46     ` Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: Jeff Law @ 2016-10-28  7:32 UTC (permalink / raw)
  To: Richard Biener, Jakub Jelinek; +Cc: Eric Botcazou, gcc-patches

On 10/28/2016 01:25 AM, Richard Biener wrote:
> On Thu, 27 Oct 2016, Jakub Jelinek wrote:
>
>> Hi!
>>
>> The following testcase ICEs on x86_64-linux with -O1, the problem is
>> that we expand assignment from COMPONENT_REF of MEM_REF into a V4SImode
>> SSA_NAME.  The MEM_REF has non-addressable DCmode var inside of it, and
>> type of a struct containing a single V4SImode element.
>>
>> The bug seems to be that if the op0 (i.e. get_inner_reference expanded)
>> is a CONCAT and we want a reference that covers all bits of the CONCAT,
>> we just short path it and return it immediately, rather than trying
>> to convert it to the requested mode.
>>
>> I've bootstrapped/regtested on x86_64-linux and i686-linux the following
>> patch, which takes the short path only if we want a complex mode.
>> The only place it makes a difference in both bootstraps/regtests was this
>> new testcase.
>>
>> Though, perhaps COMPLEX_MODE_P (mode1) is also wrong, if mode1 isn't
>> GET_MODE (op0), then we still will return something with unexpected mode
>> (e.g. DCmode vs. CDImode); I wonder if for such mismatches we shouldn't
>> just force_reg (convert_modes ()) each CONCAT operand separately and
>> create a new CONCAT.  Do we have a guarantee that COMPLEX_MODE_P (GET_MODE (op0))
>> if op0 is CONCAT?
>
> I think so.  I'll leave the rest to people more familiar with RTL
> expansion -- generally I thought the callers of expand() have to deal
> with expansions that return a different mode?
You generally have to deal with expansions that return the object in a 
new pseudo instead of the one you asked for -- so the caller has to test 
for that and emit a copy when it happens.

I don't offhand recall cases where we have to deal with getting a result 
in a different mode than was asked.  But given the history of the 
expanders, I wouldn't be surprised if there's oddball cases where that 
can happen.

jeff


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919)
  2016-10-28  7:32   ` Jeff Law
@ 2016-10-28  8:46     ` Jakub Jelinek
  2016-10-28  8:52       ` Richard Biener
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2016-10-28  8:46 UTC (permalink / raw)
  To: Jeff Law; +Cc: Richard Biener, Eric Botcazou, gcc-patches

On Fri, Oct 28, 2016 at 01:32:22AM -0600, Jeff Law wrote:
> >I think so.  I'll leave the rest to people more familiar with RTL
> >expansion -- generally I thought the callers of expand() have to deal
> >with expansions that return a different mode?
> You generally have to deal with expansions that return the object in a new
> pseudo instead of the one you asked for -- so the caller has to test for
> that and emit a copy when it happens.
> 
> I don't offhand recall cases where we have to deal with getting a result in
> a different mode than was asked.  But given the history of the expanders, I
> wouldn't be surprised if there's oddball cases where that can happen.

I've already committed the original patch based on Eric's review, but
managed to come up with another testcase that still ICEs (one with two
different complex modes).  Is the following ok for trunk if it passes
bootstrap/regtest?

2016-10-28  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/77919
	* expr.c (expand_expr_real_1) <normal_inner_ref>: Only avoid forcing
	into memory if both modes are complex and their inner modes have the
	same precision.  If the two modes are different complex modes, convert
	each part separately and generate a new CONCAT.

	* g++.dg/torture/pr77919-2.C: New test.

--- gcc/expr.c.jj	2016-10-28 10:35:14.753234774 +0200
+++ gcc/expr.c	2016-10-28 10:35:28.760057716 +0200
@@ -10422,10 +10422,35 @@ expand_expr_real_1 (tree exp, rtx target
 	  {
 	    if (bitpos == 0
 		&& bitsize == GET_MODE_BITSIZE (GET_MODE (op0))
-		&& COMPLEX_MODE_P (mode1))
+		&& COMPLEX_MODE_P (mode1)
+		&& COMPLEX_MODE_P (GET_MODE (op0))
+		&& (GET_MODE_PRECISION (GET_MODE_INNER (mode1))
+		    == GET_MODE_PRECISION (GET_MODE_INNER (GET_MODE (op0)))))
 	      {
 		if (reversep)
 		  op0 = flip_storage_order (GET_MODE (op0), op0);
+		if (mode1 != GET_MODE (op0))
+		  {
+		    rtx parts[2];
+		    for (int i = 0; i < 2; i++)
+		      {
+			rtx op = read_complex_part (op0, i != 0);
+			if (GET_CODE (op) == SUBREG)
+			  op = force_reg (GET_MODE (op), op);
+			rtx temp = gen_lowpart_common (GET_MODE_INNER (mode1),
+						       op);
+			if (temp)
+			  op = temp;
+			else
+			  {
+			    if (!REG_P (op) && !MEM_P (op))
+			      op = force_reg (GET_MODE (op), op);
+			    op = gen_lowpart (GET_MODE_INNER (mode1), op);
+			  }
+			parts[i] = op;
+		      }
+		    op0 = gen_rtx_CONCAT (mode1, parts[0], parts[1]);
+		  }
 		return op0;
 	      }
 	    if (bitpos == 0
--- gcc/testsuite/g++.dg/torture/pr77919-2.C.jj	2016-10-28 10:35:49.294798140 +0200
+++ gcc/testsuite/g++.dg/torture/pr77919-2.C	2016-10-28 10:29:38.000000000 +0200
@@ -0,0 +1,10 @@
+// PR rtl-optimization/77919
+// { dg-do compile }
+
+typedef _Complex long long B;
+struct A { A (double) {} _Complex double i; };
+typedef struct { B b; } C;
+struct D { D (const B &x) : b (x) {} B b; };
+static inline B foo (const double *x) { C *a; a = (C *) x; return a->b; }
+static inline D baz (const A &x) { return foo ((double *) &x); }
+D b = baz (0);


	Jakub

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919)
  2016-10-28  8:46     ` Jakub Jelinek
@ 2016-10-28  8:52       ` Richard Biener
  2016-10-28  8:59         ` Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: Richard Biener @ 2016-10-28  8:52 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jeff Law, Eric Botcazou, gcc-patches

On Fri, 28 Oct 2016, Jakub Jelinek wrote:

> On Fri, Oct 28, 2016 at 01:32:22AM -0600, Jeff Law wrote:
> > >I think so.  I'll leave the rest to people more familiar with RTL
> > >expansion -- generally I thought the callers of expand() have to deal
> > >with expansions that return a different mode?
> > You generally have to deal with expansions that return the object in a new
> > pseudo instead of the one you asked for -- so the caller has to test for
> > that and emit a copy when it happens.
> > 
> > I don't offhand recall cases where we have to deal with getting a result in
> > a different mode than was asked.  But given the history of the expanders, I
> > wouldn't be surprised if there's oddball cases where that can happen.
> 
> I've already committed the original patch based on Eric's review, but
> managed to come up with another testcase that still ICEs (one with two
> different complex modes).  Is the following ok for trunk if it passes
> bootstrap/regtest?

As we're dealing with memory isn't GET_MODE_SIZE the correct thing to
use?

> 2016-10-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR rtl-optimization/77919
> 	* expr.c (expand_expr_real_1) <normal_inner_ref>: Only avoid forcing
> 	into memory if both modes are complex and their inner modes have the
> 	same precision.  If the two modes are different complex modes, convert
> 	each part separately and generate a new CONCAT.
> 
> 	* g++.dg/torture/pr77919-2.C: New test.
> 
> --- gcc/expr.c.jj	2016-10-28 10:35:14.753234774 +0200
> +++ gcc/expr.c	2016-10-28 10:35:28.760057716 +0200
> @@ -10422,10 +10422,35 @@ expand_expr_real_1 (tree exp, rtx target
>  	  {
>  	    if (bitpos == 0
>  		&& bitsize == GET_MODE_BITSIZE (GET_MODE (op0))
> -		&& COMPLEX_MODE_P (mode1))
> +		&& COMPLEX_MODE_P (mode1)
> +		&& COMPLEX_MODE_P (GET_MODE (op0))
> +		&& (GET_MODE_PRECISION (GET_MODE_INNER (mode1))
> +		    == GET_MODE_PRECISION (GET_MODE_INNER (GET_MODE (op0)))))
>  	      {
>  		if (reversep)
>  		  op0 = flip_storage_order (GET_MODE (op0), op0);
> +		if (mode1 != GET_MODE (op0))
> +		  {
> +		    rtx parts[2];
> +		    for (int i = 0; i < 2; i++)
> +		      {
> +			rtx op = read_complex_part (op0, i != 0);
> +			if (GET_CODE (op) == SUBREG)
> +			  op = force_reg (GET_MODE (op), op);
> +			rtx temp = gen_lowpart_common (GET_MODE_INNER (mode1),
> +						       op);
> +			if (temp)
> +			  op = temp;
> +			else
> +			  {
> +			    if (!REG_P (op) && !MEM_P (op))
> +			      op = force_reg (GET_MODE (op), op);
> +			    op = gen_lowpart (GET_MODE_INNER (mode1), op);
> +			  }
> +			parts[i] = op;
> +		      }
> +		    op0 = gen_rtx_CONCAT (mode1, parts[0], parts[1]);
> +		  }
>  		return op0;
>  	      }
>  	    if (bitpos == 0
> --- gcc/testsuite/g++.dg/torture/pr77919-2.C.jj	2016-10-28 10:35:49.294798140 +0200
> +++ gcc/testsuite/g++.dg/torture/pr77919-2.C	2016-10-28 10:29:38.000000000 +0200
> @@ -0,0 +1,10 @@
> +// PR rtl-optimization/77919
> +// { dg-do compile }
> +
> +typedef _Complex long long B;
> +struct A { A (double) {} _Complex double i; };
> +typedef struct { B b; } C;
> +struct D { D (const B &x) : b (x) {} B b; };
> +static inline B foo (const double *x) { C *a; a = (C *) x; return a->b; }
> +static inline D baz (const A &x) { return foo ((double *) &x); }
> +D b = baz (0);
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919)
  2016-10-28  8:52       ` Richard Biener
@ 2016-10-28  8:59         ` Jakub Jelinek
  2016-10-29 16:12           ` Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2016-10-28  8:59 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jeff Law, Eric Botcazou, gcc-patches

On Fri, Oct 28, 2016 at 10:52:34AM +0200, Richard Biener wrote:
> > I've already committed the original patch based on Eric's review, but
> > managed to come up with another testcase that still ICEs (one with two
> > different complex modes).  Is the following ok for trunk if it passes
> > bootstrap/regtest?
> 
> As we're dealing with memory isn't GET_MODE_SIZE the correct thing to
> use?

GET_MODE_PRECISION is what the case VIEW_CONVERT_EXPR case tests:
      /* If the input and output modes are both the same, we are done.  */
      if (mode == GET_MODE (op0))
        ;
      /* If neither mode is BLKmode, and both modes are the same size
         then we can use gen_lowpart.  */
      else if (mode != BLKmode && GET_MODE (op0) != BLKmode
               && (GET_MODE_PRECISION (mode)
                   == GET_MODE_PRECISION (GET_MODE (op0)))
               && !COMPLEX_MODE_P (GET_MODE (op0)))  
        {
          if (GET_CODE (op0) == SUBREG)
            op0 = force_reg (GET_MODE (op0), op0);
          temp = gen_lowpart_common (mode, op0);  
          if (temp)
            op0 = temp;
          else
            {
              if (!REG_P (op0) && !MEM_P (op0))
                op0 = force_reg (GET_MODE (op0), op0);
              op0 = gen_lowpart (mode, op0);
            }
        }
The CONCAT operands can be a MEM (just likely won't be both MEM or adjacent
MEM).

BTW, the VCE part could also handle 2 different complex modes.

	Jakub

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919)
  2016-10-28  8:59         ` Jakub Jelinek
@ 2016-10-29 16:12           ` Jakub Jelinek
  2016-10-29 19:16             ` Richard Biener
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2016-10-29 16:12 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jeff Law, Eric Botcazou, gcc-patches

On Fri, Oct 28, 2016 at 10:59:35AM +0200, Jakub Jelinek wrote:
> On Fri, Oct 28, 2016 at 10:52:34AM +0200, Richard Biener wrote:
> > > I've already committed the original patch based on Eric's review, but
> > > managed to come up with another testcase that still ICEs (one with two
> > > different complex modes).  Is the following ok for trunk if it passes
> > > bootstrap/regtest?
> > 
> > As we're dealing with memory isn't GET_MODE_SIZE the correct thing to
> > use?
> 
> GET_MODE_PRECISION is what the case VIEW_CONVERT_EXPR case tests:

BTW, testing GET_MODE_SIZE or GET_MODE_BITSIZE doesn't make sense there,
            if (bitpos == 0
                && bitsize == GET_MODE_BITSIZE (GET_MODE (op0))
should already ensure that.  The GET_MODE_PRECISION check will force into
memory say x86 XCmode to {TC,CTI}mode or vice versa conversions, which are
better done through memory anyway.  If it isn't needed, then the question
is why VCE uses it.

Anyway, I've successfully bootstrapped/regtested the patch as is on
x86_64-linux and i686-linux.

	Jakub

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919)
  2016-10-29 16:12           ` Jakub Jelinek
@ 2016-10-29 19:16             ` Richard Biener
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Biener @ 2016-10-29 19:16 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jeff Law, Eric Botcazou, gcc-patches

On October 29, 2016 6:12:50 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Fri, Oct 28, 2016 at 10:59:35AM +0200, Jakub Jelinek wrote:
>> On Fri, Oct 28, 2016 at 10:52:34AM +0200, Richard Biener wrote:
>> > > I've already committed the original patch based on Eric's review,
>but
>> > > managed to come up with another testcase that still ICEs (one
>with two
>> > > different complex modes).  Is the following ok for trunk if it
>passes
>> > > bootstrap/regtest?
>> > 
>> > As we're dealing with memory isn't GET_MODE_SIZE the correct thing
>to
>> > use?
>> 
>> GET_MODE_PRECISION is what the case VIEW_CONVERT_EXPR case tests:
>
>BTW, testing GET_MODE_SIZE or GET_MODE_BITSIZE doesn't make sense
>there,
>            if (bitpos == 0
>                && bitsize == GET_MODE_BITSIZE (GET_MODE (op0))
>should already ensure that.  The GET_MODE_PRECISION check will force
>into
>memory say x86 XCmode to {TC,CTI}mode or vice versa conversions, which
>are
>better done through memory anyway.  If it isn't needed, then the
>question
>is why VCE uses it.
>
>Anyway, I've successfully bootstrapped/regtested the patch as is on
>x86_64-linux and i686-linux.

OK.

Richard.

>	Jakub


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-10-29 19:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-27 19:34 [PATCH] Fix COMPONENT_REF expansion (PR rtl-optimization/77919) Jakub Jelinek
2016-10-28  7:15 ` Eric Botcazou
2016-10-28  7:25 ` Richard Biener
2016-10-28  7:32   ` Jeff Law
2016-10-28  8:46     ` Jakub Jelinek
2016-10-28  8:52       ` Richard Biener
2016-10-28  8:59         ` Jakub Jelinek
2016-10-29 16:12           ` Jakub Jelinek
2016-10-29 19:16             ` Richard Biener

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).