Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures

2012-08-09 Thread H.J. Lu 
On Wed, Aug 8, 2012 at 8:11 AM, Richard Sandiford
rdsandif...@googlemail.com wrote:
 H.J. Lu hjl.to...@gmail.com writes:
 On Wed, Aug 8, 2012 at 6:43 AM, Uros Bizjak ubiz...@gmail.com wrote:
 Probably we need to backport this patch to 4.7, where x32 is
 -maddress-mode=long by default.


 It doesn't fail on 4.7 branch since checking mode on PLUS CONST
 is new on trunk.  However, I think it is a correctness issue.  Is this
 OK to backport to 4.7?

 Yeah, I agree we should backport it.

 Richard

I am checking this into 4.7 branch.  Tested on Linux/x32, Linux/ia32
and Linux/x86-64.

Thanks.

-- 
H.J.
---
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index bc7c36c..44b0d32 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+2012-08-09  H.J. Lu  hongjiu...@intel.com
+
+   Backport from mainline
+   2012-08-08  Richard Sandiford  rdsandif...@googlemail.com
+   H.J. Lu  hongjiu...@intel.com
+
+   PR rtl-optimization/54157
+   * combine.c (gen_lowpart_for_combine): Don't return identity
+   for CONST or symbolic reference.
+
 2012-08-06  Uros Bizjak  ubiz...@gmail.com

Backport from mainline
diff --git a/gcc/combine.c b/gcc/combine.c
index 3d81da8a..67bd776 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -10802,13 +10802,6 @@ gen_lowpart_for_combine (enum machine_mode
omode, rtx x)
   if (omode == imode)
 return x;

-  /* Return identity if this is a CONST or symbolic reference.  */
-  if (omode == Pmode
-   (GET_CODE (x) == CONST
- || GET_CODE (x) == SYMBOL_REF
- || GET_CODE (x) == LABEL_REF))
-return x;
-
   /* We can only support MODE being wider than a word if X is a
  constant integer or has a mode the same size.  */
   if (GET_MODE_SIZE (omode)  UNITS_PER_WORD
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 9fd8113..ef35a62 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2012-08-09  H.J. Lu  hongjiu...@intel.com
+
+   Backport from mainline
+   2012-08-08  H.J. Lu  hongjiu...@intel.com
+
+   PR rtl-optimization/54157
+   * gcc.target/i386/pr54157.c: New file.
+
 2012-08-01  Uros Bizjak  ubiz...@gmail.com

Backport from mainline
diff --git a/gcc/testsuite/gcc.target/i386/pr54157.c
b/gcc/testsuite/gcc.target/i386/pr54157.c
new file mode 100644
index 000..59fcd79
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr54157.c
@@ -0,0 +1,21 @@
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-options -O2 -mx32 -ftree-vectorize } */
+
+struct s2{
+  int n[24 -1][24 -1][24 -1];
+};
+
+struct test2{
+  struct s2 e;
+};
+
+struct test2 tmp2[4];
+
+void main1 ()
+{
+  int i,j;
+
+  for (i = 0; i  24 -4; i++)
+  for (j = 0; j  24 -4; j++)
+  tmp2[2].e.n[1][i][j] = 8;
+}

Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures

2012-08-08 Thread Richard Sandiford 
H.J. Lu hjl.to...@gmail.com writes:
 diff --git a/gcc/combine.c b/gcc/combine.c
 index a07c046..b9a3589 100644
 --- a/gcc/combine.c
 +++ b/gcc/combine.c
 @@ -10784,12 +10784,30 @@ gen_lowpart_for_combine (enum machine_mode omode, 
 rtx
 x)
if (omode == imode)
  return x;

 -  /* Return identity if this is a CONST or symbolic reference.  */
 -  if (omode == Pmode
 -   (GET_CODE (x) == CONST
 -   || GET_CODE (x) == SYMBOL_REF
 -   || GET_CODE (x) == LABEL_REF))
 -return x;
 +  if (omode == Pmode)
 +{
 +  /* Return identity if this is a symbolic reference.  */
 +  if (GET_CODE (x) == SYMBOL_REF || GET_CODE (x) == LABEL_REF)
 + return x;
 +
 +  /* Return identity for CONST unless this is a PLUS of 2 constant
 +  operands.  */
 +  if (GET_CODE (x) == CONST)
 + {
 +   rtx y = XEXP (x, 0);
 +   if (GET_CODE (y) == PLUS
 +((CONST_INT_P (XEXP (y, 1))
 + (GET_CODE (XEXP (y, 0)) == CONST
 +|| GET_CODE (XEXP (y, 0)) == SYMBOL_REF
 +|| GET_CODE (XEXP (y, 0)) == LABEL_REF))
 +   || (CONST_INT_P (XEXP (y, 1))
 +(GET_CODE (XEXP (y, 0)) == CONST
 +   || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
 +   || GET_CODE (XEXP (y, 0)) == LABEL_REF
 + goto fail;
 +   return x;
 + }
 +}

/* We can only support MODE being wider than a word if X is a
   constant integer or has a mode the same size.  */

 works for the testcase.

My point was that the return x is always wrong.  Whenever we return x
here, we know we're returning something in a different mode from the one
that the caller wanted.  Returning a Pmode LABEL_REF might not trigger
that plus_constant assert, but it's still wrong.

It looks like this came from the mips-rewrite branch:

2003-03-13  Richard Sandiford  rsand...@redhat.com

* combine.c (gen_lowpart_for_combine): Treat the lowpart Pmode of
a CONST as identity.  Check the return value of gen_lowpart_common.

so I can categorically confirm that the person who wrote it didn't
know what they were doing.  It also means that this case was motivated
by an experiment to make Pmode == DImode for n32, which we ended up
discarding because it produced worse code.

If this case really is important, we might consider using
convert_memory_address (Pmode, x) instead.  I'm not sure whether
that would be right for targets with address spaces though, because
we don't know which address space is associated with the address.
Hopefully someone who knows address spaces can comment.

If it is correct, then it should probably go in gen_lowpart_common
rather than gen_lowpart_for_combine.

But given how few people have hit this, it doesn't look like a
particularly important attempted optimisation.  I'll pre-approve
a patch that undoes my mistake and simply removes:

  /* Return identity if this is a CONST or symbolic reference.  */
  if (omode == Pmode
   (GET_CODE (x) == CONST
  || GET_CODE (x) == SYMBOL_REF
  || GET_CODE (x) == LABEL_REF))
return x;

Richard

Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures

2012-08-08 Thread H.J. Lu 
On Wed, Aug 8, 2012 at 1:08 AM, Richard Sandiford
rdsandif...@googlemail.com wrote:
 H.J. Lu hjl.to...@gmail.com writes:
 diff --git a/gcc/combine.c b/gcc/combine.c
 index a07c046..b9a3589 100644
 --- a/gcc/combine.c
 +++ b/gcc/combine.c
 @@ -10784,12 +10784,30 @@ gen_lowpart_for_combine (enum machine_mode omode, 
 rtx
 x)
if (omode == imode)
  return x;

 -  /* Return identity if this is a CONST or symbolic reference.  */
 -  if (omode == Pmode
 -   (GET_CODE (x) == CONST
 -   || GET_CODE (x) == SYMBOL_REF
 -   || GET_CODE (x) == LABEL_REF))
 -return x;
 +  if (omode == Pmode)
 +{
 +  /* Return identity if this is a symbolic reference.  */
 +  if (GET_CODE (x) == SYMBOL_REF || GET_CODE (x) == LABEL_REF)
 + return x;
 +
 +  /* Return identity for CONST unless this is a PLUS of 2 constant
 +  operands.  */
 +  if (GET_CODE (x) == CONST)
 + {
 +   rtx y = XEXP (x, 0);
 +   if (GET_CODE (y) == PLUS
 +((CONST_INT_P (XEXP (y, 1))
 + (GET_CODE (XEXP (y, 0)) == CONST
 +|| GET_CODE (XEXP (y, 0)) == SYMBOL_REF
 +|| GET_CODE (XEXP (y, 0)) == LABEL_REF))
 +   || (CONST_INT_P (XEXP (y, 1))
 +(GET_CODE (XEXP (y, 0)) == CONST
 +   || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
 +   || GET_CODE (XEXP (y, 0)) == LABEL_REF
 + goto fail;
 +   return x;
 + }
 +}

/* We can only support MODE being wider than a word if X is a
   constant integer or has a mode the same size.  */

 works for the testcase.

 My point was that the return x is always wrong.  Whenever we return x
 here, we know we're returning something in a different mode from the one
 that the caller wanted.  Returning a Pmode LABEL_REF might not trigger
 that plus_constant assert, but it's still wrong.

 It looks like this came from the mips-rewrite branch:

 2003-03-13  Richard Sandiford  rsand...@redhat.com

 * combine.c (gen_lowpart_for_combine): Treat the lowpart Pmode of
 a CONST as identity.  Check the return value of gen_lowpart_common.

 so I can categorically confirm that the person who wrote it didn't
 know what they were doing.  It also means that this case was motivated
 by an experiment to make Pmode == DImode for n32, which we ended up
 discarding because it produced worse code.

 If this case really is important, we might consider using
 convert_memory_address (Pmode, x) instead.  I'm not sure whether
 that would be right for targets with address spaces though, because
 we don't know which address space is associated with the address.
 Hopefully someone who knows address spaces can comment.

 If it is correct, then it should probably go in gen_lowpart_common
 rather than gen_lowpart_for_combine.

 But given how few people have hit this, it doesn't look like a
 particularly important attempted optimisation.  I'll pre-approve
 a patch that undoes my mistake and simply removes:

   /* Return identity if this is a CONST or symbolic reference.  */
   if (omode == Pmode
(GET_CODE (x) == CONST
   || GET_CODE (x) == SYMBOL_REF
   || GET_CODE (x) == LABEL_REF))
 return x;

 Richard

This is the patch I checked in.

Thanks.

-- 
H.J.
---
gcc/

2012-08-08  Richard Sandiford  rdsandif...@googlemail.com
H.J. Lu  hongjiu...@intel.com

PR rtl-optimization/54157
* combine.c (gen_lowpart_for_combine): Don't return identity
for CONST or symbolic reference.

gcc/testsuite/

2012-08-08  H.J. Lu  hongjiu...@intel.com

PR rtl-optimization/54157
* gcc.target/i386/pr54157.c: New file.

diff --git a/gcc/combine.c b/gcc/combine.c
index 495e129..2b91eb9 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -10634,13 +10634,6 @@ gen_lowpart_for_combine (enum machine_mode
omode, rtx x)
   if (omode == imode)
 return x;

-  /* Return identity if this is a CONST or symbolic reference.  */
-  if (omode == Pmode
-   (GET_CODE (x) == CONST
- || GET_CODE (x) == SYMBOL_REF
- || GET_CODE (x) == LABEL_REF))
-return x;
-
   /* We can only support MODE being wider than a word if X is a
  constant integer or has a mode the same size.  */
   if (GET_MODE_SIZE (omode)  UNITS_PER_WORD
diff --git a/gcc/testsuite/gcc.target/i386/pr54157.c
b/gcc/testsuite/gcc.target/i386/pr54157.c
new file mode 100644
index 000..b5c4528
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr54157.c
@@ -0,0 +1,21 @@
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-options -O2 -mx32 -maddress-mode=long -ftree-vectorize } */
+
+struct s2{
+  int n[24 -1][24 -1][24 -1];
+};
+
+struct test2{
+  struct s2 e;
+};
+
+struct test2 tmp2[4];
+
+void main1 ()
+{
+  int i,j;
+
+  for (i = 0; i  24 -4; i++)
+  for (j = 0; j  24 -4; j++)
+  tmp2[2].e.n[1][i][j] = 8;
+}

Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures

2012-08-08 Thread H.J. Lu 
On Wed, Aug 8, 2012 at 6:43 AM, Uros Bizjak ubiz...@gmail.com wrote:
 On Wed, Aug 8, 2012 at 3:40 PM, H.J. Lu hjl.to...@gmail.com wrote:

 rdsandif...@googlemail.com wrote:
 H.J. Lu hjl.to...@gmail.com writes:
 diff --git a/gcc/combine.c b/gcc/combine.c
 index a07c046..b9a3589 100644
 --- a/gcc/combine.c
 +++ b/gcc/combine.c
 @@ -10784,12 +10784,30 @@ gen_lowpart_for_combine (enum machine_mode 
 omode, rtx
 x)
if (omode == imode)
  return x;

 -  /* Return identity if this is a CONST or symbolic reference.  */
 -  if (omode == Pmode
 -   (GET_CODE (x) == CONST
 -   || GET_CODE (x) == SYMBOL_REF
 -   || GET_CODE (x) == LABEL_REF))
 -return x;
 +  if (omode == Pmode)
 +{
 +  /* Return identity if this is a symbolic reference.  */
 +  if (GET_CODE (x) == SYMBOL_REF || GET_CODE (x) == LABEL_REF)
 + return x;
 +
 +  /* Return identity for CONST unless this is a PLUS of 2 constant
 +  operands.  */
 +  if (GET_CODE (x) == CONST)
 + {
 +   rtx y = XEXP (x, 0);
 +   if (GET_CODE (y) == PLUS
 +((CONST_INT_P (XEXP (y, 1))
 + (GET_CODE (XEXP (y, 0)) == CONST
 +|| GET_CODE (XEXP (y, 0)) == SYMBOL_REF
 +|| GET_CODE (XEXP (y, 0)) == LABEL_REF))
 +   || (CONST_INT_P (XEXP (y, 1))
 +(GET_CODE (XEXP (y, 0)) == CONST
 +   || GET_CODE (XEXP (y, 0)) == SYMBOL_REF
 +   || GET_CODE (XEXP (y, 0)) == LABEL_REF
 + goto fail;
 +   return x;
 + }
 +}

/* We can only support MODE being wider than a word if X is a
   constant integer or has a mode the same size.  */

 works for the testcase.

 My point was that the return x is always wrong.  Whenever we return x
 here, we know we're returning something in a different mode from the one
 that the caller wanted.  Returning a Pmode LABEL_REF might not trigger
 that plus_constant assert, but it's still wrong.

 It looks like this came from the mips-rewrite branch:

 2003-03-13  Richard Sandiford  rsand...@redhat.com

 * combine.c (gen_lowpart_for_combine): Treat the lowpart Pmode of
 a CONST as identity.  Check the return value of gen_lowpart_common.

 so I can categorically confirm that the person who wrote it didn't
 know what they were doing.  It also means that this case was motivated
 by an experiment to make Pmode == DImode for n32, which we ended up
 discarding because it produced worse code.

 If this case really is important, we might consider using
 convert_memory_address (Pmode, x) instead.  I'm not sure whether
 that would be right for targets with address spaces though, because
 we don't know which address space is associated with the address.
 Hopefully someone who knows address spaces can comment.

 If it is correct, then it should probably go in gen_lowpart_common
 rather than gen_lowpart_for_combine.

 But given how few people have hit this, it doesn't look like a
 particularly important attempted optimisation.  I'll pre-approve
 a patch that undoes my mistake and simply removes:

   /* Return identity if this is a CONST or symbolic reference.  */
   if (omode == Pmode
(GET_CODE (x) == CONST
   || GET_CODE (x) == SYMBOL_REF
   || GET_CODE (x) == LABEL_REF))
 return x;

 Richard

 This is the patch I checked in.

 Probably we need to backport this patch to 4.7, where x32 is
 -maddress-mode=long by default.


It doesn't fail on 4.7 branch since checking mode on PLUS CONST
is new on trunk.  However, I think it is a correctness issue.  Is this
OK to backport to 4.7?

Thanks.

-- 
H.J.

Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures

2012-08-08 Thread Richard Sandiford 
H.J. Lu hjl.to...@gmail.com writes:
 On Wed, Aug 8, 2012 at 6:43 AM, Uros Bizjak ubiz...@gmail.com wrote:
 Probably we need to backport this patch to 4.7, where x32 is
 -maddress-mode=long by default.


 It doesn't fail on 4.7 branch since checking mode on PLUS CONST
 is new on trunk.  However, I think it is a correctness issue.  Is this
 OK to backport to 4.7?

Yeah, I agree we should backport it.

Richard

Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures

2012-08-07 Thread H.J. Lu 
On Sun, Aug 5, 2012 at 12:47 AM, Richard Sandiford
rdsandif...@googlemail.com wrote:
 For the record, I can't approve this, but...

 H.J. Lu hjl.to...@gmail.com writes:
 i386,md has

 (define_expand extzv
   [(set (match_operand:SI 0 register_operand)
 (zero_extract:SI (match_operand 1 ext_register_operand)
  (match_operand:SI 2 const8_operand)
  (match_operand:SI 3 const8_operand)))]
   

 and mode_for_extraction picks word_mode for operand 1 since
 its mode is VOIDmode.  This patch changes mode_for_extraction
 to return the mode of operand 1 if the pattern accepts any mode.
 I added *jcc_btsi_mask_2 since combine now tries a different
 pattern, which leads to test failures on gcc.target/i386/bt-mask-1.c
 and  gcc.target/i386/bt-mask-2.  I didn't update *jcc_btsi_mask_1
 instead since I am not sure if it is used elsewhere.  Tested on
 Linux/x86-64 and Linux/x32.  OK for trunk?

 the mode of the extraction operand is defined to be word_mode
 for registers (see md.texi), so that at least would need to
 be updated.  But I'm not convinced that the wanted_inner_mode here:

if (! in_dest  unsignedp
 -   mode_for_extraction (EP_extzv, -1) != MAX_MACHINE_MODE)
 +   mode_for_extraction (EP_extzv, -1, VOIDmode) != MAX_MACHINE_MODE)
  {
 -  wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1);
 -  pos_mode = mode_for_extraction (EP_extzv, 3);
 -  extraction_mode = mode_for_extraction (EP_extzv, 0);
 +  wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1,
 +  inner_mode);
 +  pos_mode = mode_for_extraction (EP_extzv, 3, VOIDmode);
 +  extraction_mode = mode_for_extraction (EP_extzv, 0, VOIDmode);
  }

 is right.  inner_mode is the mode of the thing we're extracting,
 which doesn't ncessarily have anything to do with what the ext*
 patterns support.

 FWIW, in reply to your force_to_mode message, gen_lowpart_for_combine
 looks a bit odd:

   if (omode == imode)
 return x;

   /* Return identity if this is a CONST or symbolic reference.  */
   if (omode == Pmode
(GET_CODE (x) == CONST
   || GET_CODE (x) == SYMBOL_REF
   || GET_CODE (x) == LABEL_REF))
 return x;

 So if we know the modes are different, we nevertheless return the
 original mode for CONST, SYMBOL_REF or LABEL_REF.  Surely the caller
 isn't going to be expecting that?  If we're not prepared to change
 the mode to the one that the caller asked for then I think we should
 goto fail instead.

 I don't know if you're hitting that or not.

 Richard

Yes, I did

Breakpoint 5, gen_lowpart_for_combine (omode=DImode, x=0x71b03440)
at /export/gnu/import/git/gcc-misc/gcc/combine.c:10778
10778   {
(const:SI (plus:SI (symbol_ref:SI (tmp2) var_decl 0x71997140 tmp2)
(const_int 99452 [0x1847c])))
(gdb) c
Continuing.

Breakpoint 5, gen_lowpart_for_combine (omode=DImode, x=0x71b03440)
at /export/gnu/import/git/gcc-misc/gcc/combine.c:10778
10778   {
(const:SI (plus:SI (symbol_ref:SI (tmp2) var_decl 0x71997140 tmp2)
(const_int 99452 [0x1847c])))
(gdb) bt
#0  gen_lowpart_for_combine (omode=DImode, x=0x71b03440)
at /export/gnu/import/git/gcc-misc/gcc/combine.c:10778
#1  0x00c83805 in gen_lowpart_or_truncate (x=0x71b03440,
mode=DImode) at /export/gnu/import/git/gcc-misc/gcc/combine.c:8110
#2  force_to_mode (x=0x71b02cd8, mode=DImode, mask=optimized out,
just_select=optimized out)
at /export/gnu/import/git/gcc-misc/gcc/combine.c:8389
#3  0x00c8458a in make_extraction (mode=SImode, inner=0x71b02cd8,
pos=2, pos_rtx=optimized out, len=2, unsignedp=1,
in_dest=optimized out, in_compare=0)
at /export/gnu/import/git/gcc-misc/gcc/combine.c:7480
#4  0x00c86f9c in make_compound_operation (x=x@entry=0x71b02d68,
in_code=in_code@entry=SET)
at /export/gnu/import/git/gcc-misc/gcc/combine.c:7863
#5  0x00c89924 in simplify_set (x=0x71af7c78)
at /export/gnu/import/git/gcc-misc/gcc/combine.c:6539
#6  combine_simplify_rtx (x=0x71af7c78, op0_mode=VOIDmode,
in_dest=in_dest@entry=0, in_cond=in_cond@entry=0)
at /export/gnu/import/git/gcc-misc/gcc/combine.c:5971
#7  0x00c8bd1b in subst (x=optimized out, from=optimized out,
to=optimized out, in_dest=0, in_cond=0, unique_copy=0)
at /export/gnu/import/git/gcc-misc/gcc/combine.c:5301
#8  0x00c8b8ab in subst (x=0x71aea870, from=0x71afc880,
---Type return to continue, or q return to quit---
to=0x71b02cd8, in_dest=0, in_cond=0, unique_copy=0)
at /export/gnu/import/git/gcc-misc/gcc/combine.c:5165
#9  0x00c8f875 in try_combine (i3=i3@entry=0x71afe5a0,
i2=i2@entry=0x71afe558, i1=optimized out, i0=optimized out,
i0@entry=0x0, new_direct_jump_p=new_direct_jump_p@entry=0x7fffdb64,
last_combined_insn=last_combined_insn@entry=0x71afe5a0)
at

Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures

2012-08-05 Thread Richard Sandiford 
For the record, I can't approve this, but...

H.J. Lu hjl.to...@gmail.com writes:
 i386,md has

 (define_expand extzv
   [(set (match_operand:SI 0 register_operand)
 (zero_extract:SI (match_operand 1 ext_register_operand)
  (match_operand:SI 2 const8_operand)
  (match_operand:SI 3 const8_operand)))]
   

 and mode_for_extraction picks word_mode for operand 1 since
 its mode is VOIDmode.  This patch changes mode_for_extraction
 to return the mode of operand 1 if the pattern accepts any mode.
 I added *jcc_btsi_mask_2 since combine now tries a different
 pattern, which leads to test failures on gcc.target/i386/bt-mask-1.c
 and  gcc.target/i386/bt-mask-2.  I didn't update *jcc_btsi_mask_1
 instead since I am not sure if it is used elsewhere.  Tested on
 Linux/x86-64 and Linux/x32.  OK for trunk?

the mode of the extraction operand is defined to be word_mode
for registers (see md.texi), so that at least would need to
be updated.  But I'm not convinced that the wanted_inner_mode here:

   if (! in_dest  unsignedp
-   mode_for_extraction (EP_extzv, -1) != MAX_MACHINE_MODE)
+   mode_for_extraction (EP_extzv, -1, VOIDmode) != MAX_MACHINE_MODE)
 {
-  wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1);
-  pos_mode = mode_for_extraction (EP_extzv, 3);
-  extraction_mode = mode_for_extraction (EP_extzv, 0);
+  wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1,
+  inner_mode);
+  pos_mode = mode_for_extraction (EP_extzv, 3, VOIDmode);
+  extraction_mode = mode_for_extraction (EP_extzv, 0, VOIDmode);
 }

is right.  inner_mode is the mode of the thing we're extracting,
which doesn't ncessarily have anything to do with what the ext*
patterns support.

FWIW, in reply to your force_to_mode message, gen_lowpart_for_combine
looks a bit odd:

  if (omode == imode)
return x;

  /* Return identity if this is a CONST or symbolic reference.  */
  if (omode == Pmode
   (GET_CODE (x) == CONST
  || GET_CODE (x) == SYMBOL_REF
  || GET_CODE (x) == LABEL_REF))
return x;

So if we know the modes are different, we nevertheless return the
original mode for CONST, SYMBOL_REF or LABEL_REF.  Surely the caller
isn't going to be expecting that?  If we're not prepared to change
the mode to the one that the caller asked for then I think we should
goto fail instead.

I don't know if you're hitting that or not.

Richard

Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures

2012-08-02 Thread H.J. Lu 
On Wed, Aug 1, 2012 at 12:14 PM, H.J. Lu hjl.to...@gmail.com wrote:
 On Wed, Aug 1, 2012 at 11:58 AM, Richard Sandiford
 rdsandif...@googlemail.com wrote:
 H.J. Lu hongjiu...@intel.com writes:
 We have

 (gdb) r -fpreprocessed x.i -quiet -dumpbase x.i -mx32
 -maddress-mode=long -mtune=generic -march=x86-64 -auxbase x -O2 -version
 -ftree-vectorize -o x.s
 Starting program: /export/build/gnu/gcc-x32/build-x86_64-linux/gcc/cc1
 -fpreprocessed x.i -quiet -dumpbase x.i -mx32 -maddress-mode=long
 -mtune=generic -march=x86-64 -auxbase x -O2 -version -ftree-vectorize -o
 x.s
 GNU C (GCC) version 4.8.0 20120801 (experimental)
 (x86_64-unknown-linux-gnu)
   compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
 version 5.0.2, MPFR version 3.1.0, MPC version 0.9
 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
 GNU C (GCC) version 4.8.0 20120801 (experimental)
 (x86_64-unknown-linux-gnu)
   compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
 version 5.0.2, MPFR version 3.1.0, MPC version 0.9
 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
 Compiler executable checksum: 07a4e516c4e8fe4abfdafa83737d8f4a

 Breakpoint 1, fancy_abort (
 file=0x130fe68 /export/gnu/import/git/gcc/gcc/explow.c, line=88,
 function=0x131032e __FUNCTION__.39220 plus_constant)
 at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
 1011internal_error (in %s, at %s:%d, function, trim_filename
 (file), line);
 (gdb) f 1
 #1  0x00743e07 in plus_constant (mode=DImode, x=0x7106a7e0,
 c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
 88  gcc_assert (GET_MODE (x) == VOIDmode || GET_MODE (x) == mode);
 (gdb) f 2
 #2  0x00adc4b1 in simplify_binary_operation_1 (code=PLUS,
 mode=DImode,
 op0=0x7106a7e0, op1=0x71010e80, trueop0=0x7106a7e0,
 trueop1=0x71010e80)
 at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
 1956  return plus_constant (mode, op0, INTVAL (op1));
 (gdb) call debug_rtx (op0)
 (symbol_ref:SI (tmp2) var_decl 0x70f06140 tmp2)
 (gdb) call debug_rtx (op1)
 (const_int 99452 [0x1847c])
 (gdb) bt
 #0  fancy_abort (file=0x130fe68
 /export/gnu/import/git/gcc/gcc/explow.c,
 line=88, function=0x131032e __FUNCTION__.39220 plus_constant)
 at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
 #1  0x00743e07 in plus_constant (mode=DImode, x=0x7106a7e0,
 c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
 #2  0x00adc4b1 in simplify_binary_operation_1 (code=PLUS,
 mode=DImode,
 op0=0x7106a7e0, op1=0x71010e80, trueop0=0x7106a7e0,
 trueop1=0x71010e80)
 at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
 #3  0x00adc221 in simplify_binary_operation (code=PLUS,
 mode=DImode,
 op0=0x7106a7e0, op1=0x71010e80)
 at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1904

 Things have already gone wrong by this frame: we have a DImode
 addition of an SImode value, which isn't allowed.  Where does
 that mismatch get introduced?


 make_extraction in combine generates:

 7474  inner = force_to_mode (inner, wanted_inner_mode,
 7475 pos_rtx
 7476 || len + orig_pos = 
 HOST_BITS_PER_WIDE_INT
 7477 ? ~(unsigned HOST_WIDE_INT) 0
 7478 : unsigned HOST_WIDE_INT) 1  len) 
 - 1)
 (gdb) call debug_rtx (inner)
 (plus:SI (reg:SI 109 [ D.1765 ])
 (const:SI (plus:SI (symbol_ref:SI (tmp2) var_decl 0x70f06140 tmp2)
 (const_int 99452 [0x1847c]
 (gdb) p wanted_inner_mode
 $2 = DImode
 (gdb)


i386,md has

(define_expand extzv
  [(set (match_operand:SI 0 register_operand)
(zero_extract:SI (match_operand 1 ext_register_operand)
 (match_operand:SI 2 const8_operand)
 (match_operand:SI 3 const8_operand)))]
  

and mode_for_extraction picks word_mode for operand 1 since
its mode is VOIDmode.  This patch changes mode_for_extraction
to return the mode of operand 1 if the pattern accepts any mode.
I added *jcc_btsi_mask_2 since combine now tries a different
pattern, which leads to test failures on gcc.target/i386/bt-mask-1.c
and  gcc.target/i386/bt-mask-2.  I didn't update *jcc_btsi_mask_1
instead since I am not sure if it is used elsewhere.  Tested on
Linux/x86-64 and Linux/x32.  OK for trunk?

Thanks.

-- 
H.J.

gcc/

2012-08-02  H.J. Lu  hongjiu...@intel.com

PR rtl-optimization/54157
* combine.c (make_extraction): Pass inner_mode to mode_for_extraction
for operand 1, otherwise pass VOIDmode.
(simplify_comparison): Pass VOIDmode to mode_for_extraction.

* expmed.c (mode_for_extraction): Add a machine_mode argument
and use it instead of word_mode if it isn't VOIDmode.
(store_bit_field_1): Pass VOIDmode to mode_for_extraction.
(store_bit_field): Likewise.

PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures

2012-08-01 Thread H.J. Lu 
Hi,

We have

(gdb) r -fpreprocessed x.i -quiet -dumpbase x.i -mx32
-maddress-mode=long -mtune=generic -march=x86-64 -auxbase x -O2 -version
-ftree-vectorize -o x.s
Starting program: /export/build/gnu/gcc-x32/build-x86_64-linux/gcc/cc1
-fpreprocessed x.i -quiet -dumpbase x.i -mx32 -maddress-mode=long
-mtune=generic -march=x86-64 -auxbase x -O2 -version -ftree-vectorize -o
x.s
GNU C (GCC) version 4.8.0 20120801 (experimental)
(x86_64-unknown-linux-gnu)
compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
version 5.0.2, MPFR version 3.1.0, MPC version 0.9
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
GNU C (GCC) version 4.8.0 20120801 (experimental)
(x86_64-unknown-linux-gnu)
compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
version 5.0.2, MPFR version 3.1.0, MPC version 0.9
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
Compiler executable checksum: 07a4e516c4e8fe4abfdafa83737d8f4a

Breakpoint 1, fancy_abort (
file=0x130fe68 /export/gnu/import/git/gcc/gcc/explow.c, line=88, 
function=0x131032e __FUNCTION__.39220 plus_constant)
at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
1011  internal_error (in %s, at %s:%d, function, trim_filename
(file), line);
(gdb) f 1
#1  0x00743e07 in plus_constant (mode=DImode, x=0x7106a7e0, 
c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
88gcc_assert (GET_MODE (x) == VOIDmode || GET_MODE (x) == mode);
(gdb) f 2
#2  0x00adc4b1 in simplify_binary_operation_1 (code=PLUS,
mode=DImode, 
op0=0x7106a7e0, op1=0x71010e80, trueop0=0x7106a7e0, 
trueop1=0x71010e80)
at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
1956return plus_constant (mode, op0, INTVAL (op1));
(gdb) call debug_rtx (op0)
(symbol_ref:SI (tmp2) var_decl 0x70f06140 tmp2)
(gdb) call debug_rtx (op1)
(const_int 99452 [0x1847c])
(gdb) bt
#0  fancy_abort (file=0x130fe68
/export/gnu/import/git/gcc/gcc/explow.c, 
line=88, function=0x131032e __FUNCTION__.39220 plus_constant)
at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
#1  0x00743e07 in plus_constant (mode=DImode, x=0x7106a7e0, 
c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
#2  0x00adc4b1 in simplify_binary_operation_1 (code=PLUS,
mode=DImode, 
op0=0x7106a7e0, op1=0x71010e80, trueop0=0x7106a7e0, 
trueop1=0x71010e80)
at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
#3  0x00adc221 in simplify_binary_operation (code=PLUS,
mode=DImode, 
op0=0x7106a7e0, op1=0x71010e80)
at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1904
#4  0x00ae4beb in simplify_plus_minus (code=PLUS, mode=DImode, 
op0=0x71071d80, op1=0x71072440)
at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:4083
#5  0x00adcd81 in simplify_binary_operation_1 (code=PLUS,
mode=DImode, 
op0=0x71071d80, op1=0x71072440, trueop0=0x71071d80, 
trueop1=0x71072440)
at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:2079
#6  0x00adc221 in simplify_binary_operation (code=PLUS,
mode=DImode, 
op0=0x71071d80, op1=0x71072440)
at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1904
#7  0x00ad6f55 in simplify_gen_binary (code=PLUS, mode=DImode, 
---Type return to continue, or q return to quit---
op0=0x71071d80, op1=0x71072440)
at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:173
#8  0x0111b868 in force_to_mode (x=0x71071cd8, mode=DImode, 
mask=15, just_select=0) at
/export/gnu/import/git/gcc/gcc/combine.c:8392
#9  0x01118ed6 in make_extraction (mode=SImode,
inner=0x71071cd8, 
pos=2, pos_rtx=0x0, len=2, unsignedp=1, in_dest=0, in_compare=0)
at /export/gnu/import/git/gcc/gcc/combine.c:7474
#10 0x01119981 in make_compound_operation (x=0x71071d68, 
in_code=SET) at /export/gnu/import/git/gcc/gcc/combine.c:7721
#11 0x01116cc5 in simplify_set (x=0x71066c78)
at /export/gnu/import/git/gcc/gcc/combine.c:6539
#12 0x01114e91 in combine_simplify_rtx (x=0x71066c78, 
op0_mode=VOIDmode, in_dest=0, in_cond=0)
at /export/gnu/import/git/gcc/gcc/combine.c:5971
#13 0x01113250 in subst (x=0x71066c78, from=0x7106a860, 
to=0x71071cd8, in_dest=0, in_cond=0, unique_copy=0)
at /export/gnu/import/git/gcc/gcc/combine.c:5301
#14 0x01112d30 in subst (x=0x71059870, from=0x7106a860, 
to=0x71071cd8, in_dest=0, in_cond=0, unique_copy=0)
at /export/gnu/import/git/gcc/gcc/combine.c:5164
#15 0x0110d1e3 in try_combine (i3=0x7106c5a0,
i2=0x7106c558, 
i1=0x7106c510, i0=0x0, new_direct_jump_p=0x7fffd974, 
last_combined_insn=0x7106c5a0)
---Type return to continue, or q return to quit---
at /export/gnu/import/git/gcc/gcc/combine.c:3259
#16 0x011083ad in combine_instructions (f=0x7102ba40,

Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures

2012-08-01 Thread Richard Sandiford 
H.J. Lu hongjiu...@intel.com writes:
 We have

 (gdb) r -fpreprocessed x.i -quiet -dumpbase x.i -mx32
 -maddress-mode=long -mtune=generic -march=x86-64 -auxbase x -O2 -version
 -ftree-vectorize -o x.s
 Starting program: /export/build/gnu/gcc-x32/build-x86_64-linux/gcc/cc1
 -fpreprocessed x.i -quiet -dumpbase x.i -mx32 -maddress-mode=long
 -mtune=generic -march=x86-64 -auxbase x -O2 -version -ftree-vectorize -o
 x.s
 GNU C (GCC) version 4.8.0 20120801 (experimental)
 (x86_64-unknown-linux-gnu)
   compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
 version 5.0.2, MPFR version 3.1.0, MPC version 0.9
 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
 GNU C (GCC) version 4.8.0 20120801 (experimental)
 (x86_64-unknown-linux-gnu)
   compiled by GNU C version 4.7.1 20120629 (Red Hat 4.7.1-1), GMP
 version 5.0.2, MPFR version 3.1.0, MPC version 0.9
 GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
 Compiler executable checksum: 07a4e516c4e8fe4abfdafa83737d8f4a

 Breakpoint 1, fancy_abort (
 file=0x130fe68 /export/gnu/import/git/gcc/gcc/explow.c, line=88, 
 function=0x131032e __FUNCTION__.39220 plus_constant)
 at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
 1011internal_error (in %s, at %s:%d, function, trim_filename
 (file), line);
 (gdb) f 1
 #1  0x00743e07 in plus_constant (mode=DImode, x=0x7106a7e0, 
 c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
 88  gcc_assert (GET_MODE (x) == VOIDmode || GET_MODE (x) == mode);
 (gdb) f 2
 #2  0x00adc4b1 in simplify_binary_operation_1 (code=PLUS,
 mode=DImode, 
 op0=0x7106a7e0, op1=0x71010e80, trueop0=0x7106a7e0, 
 trueop1=0x71010e80)
 at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
 1956  return plus_constant (mode, op0, INTVAL (op1));
 (gdb) call debug_rtx (op0)
 (symbol_ref:SI (tmp2) var_decl 0x70f06140 tmp2)
 (gdb) call debug_rtx (op1)
 (const_int 99452 [0x1847c])
 (gdb) bt
 #0  fancy_abort (file=0x130fe68
 /export/gnu/import/git/gcc/gcc/explow.c, 
 line=88, function=0x131032e __FUNCTION__.39220 plus_constant)
 at /export/gnu/import/git/gcc/gcc/diagnostic.c:1011
 #1  0x00743e07 in plus_constant (mode=DImode, x=0x7106a7e0, 
 c=99452) at /export/gnu/import/git/gcc/gcc/explow.c:88
 #2  0x00adc4b1 in simplify_binary_operation_1 (code=PLUS,
 mode=DImode, 
 op0=0x7106a7e0, op1=0x71010e80, trueop0=0x7106a7e0, 
 trueop1=0x71010e80)
 at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1956
 #3  0x00adc221 in simplify_binary_operation (code=PLUS,
 mode=DImode, 
 op0=0x7106a7e0, op1=0x71010e80)
 at /export/gnu/import/git/gcc/gcc/simplify-rtx.c:1904

Things have already gone wrong by this frame: we have a DImode
addition of an SImode value, which isn't allowed.  Where does
that mismatch get introduced?

Richard