RE: [PATCH, ARM, iWMMXT] Check IWMMXT_GR_REGS in the SECONDARY_RELOAD MACRO

2013-06-19 Thread Xinyu Qi
At 2013-05-24 15:19:36,Chung-Ju Wu jasonw...@gmail.com wrote: 
 2013/5/24 Xinyu Qi x...@marvell.com:
  Hi,
 
For this simple case, compiled with option -march=iwmmxt -O, #define
  N 64 signed int b[N]; signed long long j[N], d[N]; void foo (void) {
int i;
for (i = 0; i  N; i++)
  j[i] = d[i]  b[i];
  }
  An internal compiler error occurs,
  error: insn does not satisfy its constraints:
  (insn 112 74 75 3 (set (reg:SI 96 wcgr0)
  (mem/c:SI (plus:SI (reg:SI 3 r3 [orig:174 ivtmp.19 ] [174])
  (reg/f:SI 0 r0 [183])) [0 MEM[symbol: b, index:
 ivtmp.19_14, offset: 0B]+0 S4 A32])) {*iwmmxt_movsi_insn}
   (nil))
 
  The load address of wmmx wcgr register cannot accept the register offset
 mode and the reload pass fails to fix it, so that such error happens.
  This issue could be solved by adding check code for IWMMXT_GR_REGS class
 in the SECONDARY_RELOAD MACRO if TARGET_IWMMXT. Current code only
 check the IWMMXT_REGS group.
  Patch attached with a new test.
  Pass full dejagnu test. No regression.
 
  Is this fix proper?
  OK for trunk?
 
 
 I cannot approve it.  But here are some comments and hope it helps.

Hi Chung-Ju,

Thanks for your comments:)
I fixed the typo you mentioned and regenerated the patch attached.

ChangeLog
gcc/
2013-05-24  Xinyu Qi  x...@marvell.com

* config/arm/arm.h (SECONDARY_OUTPUT_RELOAD_CLASS): Check
IWMMXT_GR_REGS register class.
(SECONDARY_INPUT_RELOAD_CLASS): Likewise.

testsuite/
2013-05-24  Xinyu Qi  x...@marvell.com

* gcc.target/arm/mmx-3.c: New test.

Thanks,
Xinyu

 
 
  ChangeLog
  gcc/
  2013-05-24  Xinyu Qi  x...@marvell.com
 
  * config/arm/arm.h (SECONDARY_OUTPUT_RELOAD_CLASS):
 Check IWMMXT_GR_REGS.
 
 This line just ends at 81 column.
 How about this one?
 
 2013-05-24  Xinyu Qi  x...@marvell.com
 
 * config/arm/arm.h (SECONDARY_OUTPUT_RELOAD_CLASS): Check
 IWMMXT_GR_REGS register class.
 (SECONDARY_INPUT_RELOAD_CLASS): Likewise.
 
 
  testsuite/
  2013-05-24  Xinyu Qi  x...@marvell.com
 
  * gcc.target/arm/mmx-3.c: New test.
 
 
  Index: gcc/config/arm/arm.h
 
 
 ===
  --- gcc/config/arm/arm.h(revision 199090)
  +++ gcc/config/arm/arm.h(working copy)
  @@ -1280,7 +1280,8 @@
 ((TARGET_VFP  TARGET_HARD_FLOAT\
IS_VFP_CLASS (CLASS))\
  ? coproc_secondary_reload_class (MODE, X, FALSE)\
  -   : (TARGET_IWMMXT  (CLASS) == IWMMXT_REGS)\
  +   : (TARGET_IWMMXT  ((CLASS) == IWMMXT_REGS)\
  +|| (CLASS) == IWMMXT_GR_REGS)\
 
 I think it should be
 
 +   : (TARGET_IWMMXT  ((CLASS) == IWMMXT_REGS\
 +|| (CLASS) == IWMMXT_GR_REGS))\
 
 
  ? coproc_secondary_reload_class (MODE, X, TRUE)\
  : TARGET_32BIT\
  ? (((MODE) == HImode  ! arm_arch4  true_regnum (X) == -1) \
 @@
  -1293,7 +1294,8 @@
 ((TARGET_VFP  TARGET_HARD_FLOAT\
IS_VFP_CLASS (CLASS))\
   ? coproc_secondary_reload_class (MODE, X, FALSE) :\
  -(TARGET_IWMMXT  (CLASS) == IWMMXT_REGS) ?\
  +(TARGET_IWMMXT  ((CLASS) == IWMMXT_REGS\
  +   || (CLASS) == IWMMXT_GR_REGS)) ?\
   coproc_secondary_reload_class (MODE, X, TRUE) :\
  (TARGET_32BIT ?\
   (((CLASS) == IWMMXT_REGS || (CLASS) == IWMMXT_GR_REGS)\
 
 It seems that you didn't CC arm maintainer.
 Let me do this for you. :)
 
 
 Best regards,
 jasonwucj


GR_secondary_reload.diff
Description: GR_secondary_reload.diff


Re: [PATCH, ARM, iWMMXT] Check IWMMXT_GR_REGS in the SECONDARY_RELOAD MACRO

2013-05-24 Thread Chung-Ju Wu
2013/5/24 Xinyu Qi x...@marvell.com:
 Hi,

   For this simple case, compiled with option -march=iwmmxt -O,
 #define N 64
 signed int b[N];
 signed long long j[N], d[N];
 void foo (void)
 {
   int i;
   for (i = 0; i  N; i++)
 j[i] = d[i]  b[i];
 }
 An internal compiler error occurs,
 error: insn does not satisfy its constraints:
 (insn 112 74 75 3 (set (reg:SI 96 wcgr0)
 (mem/c:SI (plus:SI (reg:SI 3 r3 [orig:174 ivtmp.19 ] [174])
 (reg/f:SI 0 r0 [183])) [0 MEM[symbol: b, index: ivtmp.19_14, 
 offset: 0B]+0 S4 A32])) {*iwmmxt_movsi_insn}
  (nil))

 The load address of wmmx wcgr register cannot accept the register offset mode 
 and the reload pass fails to fix it, so that such error happens.
 This issue could be solved by adding check code for IWMMXT_GR_REGS class in 
 the SECONDARY_RELOAD MACRO if TARGET_IWMMXT. Current code only check the 
 IWMMXT_REGS group.
 Patch attached with a new test.
 Pass full dejagnu test. No regression.

 Is this fix proper?
 OK for trunk?


I cannot approve it.  But here are some comments and hope it helps.


 ChangeLog
 gcc/
 2013-05-24  Xinyu Qi  x...@marvell.com

 * config/arm/arm.h (SECONDARY_OUTPUT_RELOAD_CLASS): Check 
 IWMMXT_GR_REGS.

This line just ends at 81 column.
How about this one?

2013-05-24  Xinyu Qi  x...@marvell.com

* config/arm/arm.h (SECONDARY_OUTPUT_RELOAD_CLASS): Check
IWMMXT_GR_REGS register class.
(SECONDARY_INPUT_RELOAD_CLASS): Likewise.


 testsuite/
 2013-05-24  Xinyu Qi  x...@marvell.com

 * gcc.target/arm/mmx-3.c: New test.


 Index: gcc/config/arm/arm.h
 ===
 --- gcc/config/arm/arm.h(revision 199090)
 +++ gcc/config/arm/arm.h(working copy)
 @@ -1280,7 +1280,8 @@
((TARGET_VFP  TARGET_HARD_FLOAT\
   IS_VFP_CLASS (CLASS))\
 ? coproc_secondary_reload_class (MODE, X, FALSE)\
 -   : (TARGET_IWMMXT  (CLASS) == IWMMXT_REGS)\
 +   : (TARGET_IWMMXT  ((CLASS) == IWMMXT_REGS)\
 +|| (CLASS) == IWMMXT_GR_REGS)\

I think it should be

+   : (TARGET_IWMMXT  ((CLASS) == IWMMXT_REGS\
+|| (CLASS) == IWMMXT_GR_REGS))\


 ? coproc_secondary_reload_class (MODE, X, TRUE)\
 : TARGET_32BIT\
 ? (((MODE) == HImode  ! arm_arch4  true_regnum (X) == -1) \
 @@ -1293,7 +1294,8 @@
((TARGET_VFP  TARGET_HARD_FLOAT\
   IS_VFP_CLASS (CLASS))\
  ? coproc_secondary_reload_class (MODE, X, FALSE) :\
 -(TARGET_IWMMXT  (CLASS) == IWMMXT_REGS) ?\
 +(TARGET_IWMMXT  ((CLASS) == IWMMXT_REGS\
 +   || (CLASS) == IWMMXT_GR_REGS)) ?\
  coproc_secondary_reload_class (MODE, X, TRUE) :\
 (TARGET_32BIT ?\
  (((CLASS) == IWMMXT_REGS || (CLASS) == IWMMXT_GR_REGS)\

It seems that you didn't CC arm maintainer.
Let me do this for you. :)


Best regards,
jasonwucj