[Bug target/55426] New: gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb

[Bug target/55426] New: gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb

[mmatti.gcc at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55426

             Bug #: 55426
           Summary: gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: mmatti.gcc@gmail.com
                CC: christophe.lyon@st.com, ramana.radhakrishnan@arm.com
              Host: Scientific Linux 6.3
            Target: arm big-endian
             Build: Scientific Linux 6.3


Hi,

I have encountered a following ICE on arm big endian target

{{{
neon-vld1_dupQ.c: In function ‘main’:
neon-vld1_dupQ.c:24:1: error: unrecognizable insn:
(insn 94 93 31 (set (subreg:DI (reg:V2DI 95 d16 [orig:137 D.14400 ] [137]) 0)
        (subreg:DI (reg:V2DI 95 d16 [orig:137 D.14400 ] [137]) 8))
/home/armbe/toolchain/armeb-tools/bin/../lib/gcc/armeb-none-linux-gnueabi/4.7.0/include/arm_neon.h:8214
-1
     (nil))
neon-vld1_dupQ.c:24:1: internal compiler error: in extract_insn, at
recog.c:2123
Please submit a full bug report,
}}}

My initial investigation shows that the following patch (split insn)
http://gcc.gnu.org/ml/gcc-patches/2012-05/msg01131.html is causing
this failure.
I think the CANNOT_CHANGE_MODE_CLASS macro is present and also plays a
role in not allowing the mov of DImode in this case.

If I revert the patch then I do not encounter this problem.

I can propose a solution, which may fix this problem.

In case of little endian the compiler generates,

{{{
insn 93 26 94 (set (reg:DI 95 d16 [orig:137 D.14400 ] [137])
        (vec_duplicate:DI (mem:DI (reg/f:SI 2 r2 [155]) [0 *(const
__builtin_neon_di[1] *) __a_5+0 S8 A64])))
/home/armbe/toolchain/arm-tools/bin/../lib/gcc/arm-montavista-linux-gnueabi/4.7.0/include/arm_neon.h:8214
1606 {neon_vld1_dupdi}
     (nil))
        vld1.64 {d16}, [r2:64]  @ 93    neon_vld1_dupdi [length = 4]
(insn 94 93 31 (set (reg:DI 97 d17 [ D.14400+8 ])
        (reg:DI 95 d16 [orig:137 D.14400 ] [137]))
/home/armbe/toolchain/arm-tools/bin/../lib/
}}}

But incase of big endian the vmov\t%f0, %e0 is missing there is no
pattern in the neon.md file.
Adding this may solve the problem. Something like this

(define_insn "*neon_subreg_di"
  [(set (match_operand:DI 0 "s_register_operand" "=w")
       (match_operand:DI 1 "s_register_operand" "w"))]
 "TARGET_NEON"
 "vmov\t%1, %0"
)

Kindly comment on this patch or could you please help me with the 
pointers to solve this problem.

Regards,
Manjunath S Matti.

[Bug target/55426] gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb

[christophe.lyon at st dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55426

--- Comment #1 from christophe.lyon at st dot com 2012-11-21 15:50:14 UTC ---
Reverting the definition of CANNOT_CHANGE_MODE_CLASS introduced on 2012-10-22
at rev #192687 avoids the ICE too.

[Bug target/55426] gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb

[mmatti.gcc at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55426

--- Comment #2 from Manjunath S Matti <mmatti.gcc at gmail dot com> 2012-11-27 13:43:43 UTC ---
(In reply to comment #1)
> Reverting the definition of CANNOT_CHANGE_MODE_CLASS introduced on 2012-10-22
> at rev #192687 avoids the ICE too.

I have tried this, reverting the revision #192687 would fix this issue. But as 
the comment of the patch specifies 
"In big-endian mode, modes greater than word size (i.e. DFmode) are stored in
VFP registers in little-endian order.  We can't describe that accurately to
GCC, so avoid taking subregs of such values.".

Reverting the patch would adversely affect the big-endian toolchain. Hence, I
assume that we should have an alternate fix.

[Bug target/55426] gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb

[ramana at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55426

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ramana at gcc dot gnu.org

--- Comment #3 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> 2012-11-28 03:14:03 UTC ---
(In reply to comment #2)
> (In reply to comment #1)
> > Reverting the definition of CANNOT_CHANGE_MODE_CLASS introduced on 2012-10-22
> > at rev #192687 avoids the ICE too.
> 
> I have tried this, reverting the revision #192687 would fix this issue. But as 
> the comment of the patch specifies 
> "In big-endian mode, modes greater than word size (i.e. DFmode) are stored in
> VFP registers in little-endian order.  We can't describe that accurately to
> GCC, so avoid taking subregs of such values.".
> 
> Reverting the patch would adversely affect the big-endian toolchain. Hence, I
> assume that we should have an alternate fix.
(In reply to comment #2)
> (In reply to comment #1)
> > Reverting the definition of CANNOT_CHANGE_MODE_CLASS introduced on 2012-10-22
> > at rev #192687 avoids the ICE too.
> 
> I have tried this, reverting the revision #192687 would fix this issue. But as 
> the comment of the patch specifies 
> "In big-endian mode, modes greater than word size (i.e. DFmode) are stored in
> VFP registers in little-endian order.  We can't describe that accurately to
> GCC, so avoid taking subregs of such values.".
> 
> Reverting the patch would adversely affect the big-endian toolchain. Hence, I
> assume that we should have an alternate fix.

We do have w-w moves in vfp.md *movdi_vfp and friends. why doesn't that work
for you ?

Ramana

[Bug target/55426] gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb

[mmatti.gcc at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55426

--- Comment #4 from Manjunath S Matti <mmatti.gcc at gmail dot com> 2012-11-30 09:26:04 UTC ---
Created attachment 28836
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28836
Proposed fix

[Bug target/55426] gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb

[mmatti.gcc at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55426

--- Comment #5 from Manjunath S Matti <mmatti.gcc at gmail dot com> 2012-11-30 09:28:04 UTC ---
The "define_insn_and_split "neon_vld1_dupv2di" contains the DI move pattern 
as second instruction in split. The pattern generated for the move are 
subreg:DI (Reg:VDI) to subreg:DI (Reg:VDI).


Ideally, this pattern should use "*movdi_vfp" in vfp.md file as suggested by 
Ramana. 

However, the "*movdi_vfp" specifies that at least one of the operands should be
placed in register. So, subreg in our move pattern should satisfy the
condition.
However, the condition is not  satisfied and hence the move pattern is not
being
recognized for DImode.

Hence, to allow the register in one of the operands, "neon_vld1_dupv2di"
pattern
was modified to use register in one of the operand. 

Reload condition was removed as a pseudo register is used in one of the
operands. This modification fixed the ICE.

Tested with no extra regressions.

[Bug target/55426] gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb

[ktkachov at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55426

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2014-02-25
                 CC|                            |ktkachov at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |ktkachov at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #6 from ktkachov at gcc dot gnu.org ---
Mine.

[Bug target/55426] gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb

[ktkachov at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55426

--- Comment #7 from ktkachov at gcc dot gnu.org ---
Author: ktkachov
Date: Tue Feb 25 10:20:40 2014
New Revision: 208116

URL: http://gcc.gnu.org/viewcvs?rev=208116&root=gcc&view=rev
Log:
    PR target/55426
    * config/arm/arm.h (CANNOT_CHANGE_MODE_CLASS): Allow 128 to 64-bit
    conversions.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.h

[Bug target/55426] gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb

[ktkachov at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55426

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from ktkachov at gcc dot gnu.org ---
Fixed on 4.9 trunk and 4.8

[Bug target/55426] gcc.target/arm/neon-vld1_dupQ.c ICEs on armeb

[ktkachov at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55426

--- Comment #8 from ktkachov at gcc dot gnu.org ---
Author: ktkachov
Date: Tue Feb 25 10:25:26 2014
New Revision: 208117

URL: http://gcc.gnu.org/viewcvs?rev=208117&root=gcc&view=rev
Log:
    PR target/55426
    * config/arm/arm.h (CANNOT_CHANGE_MODE_CLASS): Allow 128 to 64-bit
    conversions.

Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/arm/arm.h