[Bug c/58314] New: SH4 error: 'asm' operand requires impossible reload

[Bug c/58314] New: SH4 error: 'asm' operand requires impossible reload

[chrbr at gcc dot gnu.org]

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

            Bug ID: 58314
           Summary: SH4 error: 'asm' operand requires impossible reload
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: chrbr at gcc dot gnu.org

Created attachment 30746
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30746&action=edit
Reproduce

Reproduced in -Os with 
 - sh4-linux-gcc 4.8.1 (sh-superh-elf-gcc surprisingly OK)
 - sh-superh-elf-gcc 4.9 

sh-superh-elf-gcc -Os bug_asm.c

bug_asm.c: In function 'xfs_attr_leaf_remove':
bug_asm.c:206:2: error: 'asm' operand requires impossible reload
  __asm__(
  ^

See similar reports :  #10396, #13515, #48496. Does not look similar as still
not fixed and pertained to older versions.

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[chrbr at gcc dot gnu.org]

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

chrbr at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |target

--- Comment #1 from chrbr at gcc dot gnu.org ---
Indeed, not directly related to the asm, we end-up generating a pattern

(insn 195 194 64 2 (set (reg:HI 261 [ x ])
        (reg:HI 0 r0)) pr58314.c:12 261 {*movhi_reg_reg}
     (nil))

with reg 261 that can't be reloaded and fails the constrain_operands in

          if (asm_noperands (PATTERN (insn)) >= 0)
        for (p = NEXT_INSN (prev); p != next; p = NEXT_INSN (p))
          if (p != insn && INSN_P (p)
              && GET_CODE (PATTERN (p)) != USE
              && (recog_memoized (p) < 0
              || (extract_insn (p), ! constrain_operands (1))))
            {
              error_for_asm (insn,
                     "%<asm%> operand requires "
                     "impossible reload");
              delete_insn (p);
            }
        }

the pattern movhi_reg_reg not accepting memory reload. This is a regression
this the constraints we removed from the *movhi patten (that accepted memory
spills)

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[chrbr at gcc dot gnu.org]

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

--- Comment #2 from chrbr at gcc dot gnu.org ---
Author: chrbr
Date: Fri Sep 13 07:51:07 2013
New Revision: 202557

URL: http://gcc.gnu.org/viewcvs?rev=202557&root=gcc&view=rev
Log:
2013-09-13  Christian Bruel  <christian.bruel@st.com>

        PR target/58314
        * config/sh/sh.md (mov<mode>_reg_reg): Allow memory reloads.


Added:
    trunk/gcc/testsuite/gcc.target/sh/torture/pr58314.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh.md
    trunk/gcc/testsuite/ChangeLog

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[chrbr at gcc dot gnu.org]

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

--- Comment #3 from chrbr at gcc dot gnu.org ---
Author: chrbr
Date: Fri Sep 13 08:38:22 2013
New Revision: 202559

URL: http://gcc.gnu.org/viewcvs?rev=202559&root=gcc&view=rev
Log:
2013-09-13  Christian Bruel  <christian.bruel@st.com>

        PR target/58314
        * config/sh/sh.md (mov<mode>_reg_reg): Allow memory reloads.


Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/sh/torture/pr58314.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/sh/sh.md
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[chrbr at gcc dot gnu.org]

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

chrbr at gcc dot gnu.org changed:

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

--- Comment #4 from chrbr at gcc dot gnu.org ---
Fixed for 4.8 and 4.9 branches

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[chrbr at gcc dot gnu.org]

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

chrbr at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
   Last reconfirmed|                            |2013-11-20
                 CC|                            |olegendo at gcc dot gnu.org
         Resolution|FIXED                       |---
     Ever confirmed|0                           |1

--- Comment #5 from chrbr at gcc dot gnu.org ---
Linux kernel build fails since 4.8

cc1 -O2 consolemap.c 

drivers/char/consolemap.c:654:647: error: 'asm' operand requires impossible
reload

seems to be due to this movhi<mode>_reg_reg split out of the *movhi insns.

Oleg, I think it time to re-unify those. Doing an experimental resurrection of
the r,r reload constraints seems to fix it, but without knowing the impacts on
your T-bit combine optimizations...

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[chrbr at gcc dot gnu.org]

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

--- Comment #6 from chrbr at gcc dot gnu.org ---
Created attachment 31257
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31257&action=edit
test case

cc1 -O2 consolemap.c -quiet

drivers/char/consolemap.c:654:647: error: 'asm' operand requires impossible
reload

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[olegendo at gcc dot gnu.org]

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

--- Comment #7 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to chrbr from comment #5)
> Linux kernel build fails since 4.8
> 
> cc1 -O2 consolemap.c 
> 
> drivers/char/consolemap.c:654:647: error: 'asm' operand requires impossible
> reload
> 
> seems to be due to this movhi<mode>_reg_reg split out of the *movhi insns.
> 
> Oleg, I think it time to re-unify those. Doing an experimental resurrection
> of the r,r reload constraints seems to fix it, but without knowing the
> impacts on your T-bit combine optimizations...

OK, I'll try to have a look at it within the next couple of days.

The T-bit combine stuff shouldn't be affected by that at all.
If anything, then it would be the byte/word displacement addressing stuff (PR
50751).

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[olegendo at gcc dot gnu.org]

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

--- Comment #8 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Created attachment 31260
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31260&action=edit
reduced test case

(In reply to chrbr from comment #6)
> Created attachment 31257 [details]
> test case
> 
> cc1 -O2 consolemap.c -quiet
> 
> drivers/char/consolemap.c:654:647: error: 'asm' operand requires impossible
> reload

A reduced test case of the above, suitable for inclusion into e.g.
gcc/testsuite/gcc.target/sh/torture

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[olegendo at gcc dot gnu.org]

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

--- Comment #10 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #9)
> 
> I'm testing the above patch now.

And there are failures.  Here is one (I think I remember it when working on the
QI/HImode displacement addressing stuff):

FAIL: gcc.dg/torture/vshuf-v16hi.c  -O2  (test for excess errors)
Excess errors:
/usr/local/sh-elf/bin/ld: internal error: merge of architecture 'sh3e' with
architecture 'sh2a-nofpu' produced unknown architecture
/usr/local/sh-elf/bin/ld: /tmp/ccRPtSqs.o: uses instructions which are
incompatible with instructions used in previous modules
/usr/local/sh-elf/bin/ld: failed to merge target specific data of file
/tmp/ccRPtSqs.o

This usually happens because SH2A insns are output -- 32 bit displacement
addressing insns which are more flexible -- even though the target is non-SH2A.
 This is what the comment above the reg_reg pattern is talking about.

(There is no compiler error because the target type is not passed down to the
assembler by the compiler).

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[olegendo at gcc dot gnu.org]

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

--- Comment #11 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #10)
> 
> This usually happens because SH2A insns are output -- 32 bit displacement
> addressing insns which are more flexible -- even though the target is
> non-SH2A.  This is what the comment above the reg_reg pattern is talking
> about.
> 
> (There is no compiler error because the target type is not passed down to
> the assembler by the compiler).

This patch can be applied to enable the error message by the assembler:

Index: gcc/config/sh/sh.h
===================================================================
--- gcc/config/sh/sh.h    (revision 205190)
+++ gcc/config/sh/sh.h    (working copy)
@@ -267,9 +267,25 @@
 #define SUBTARGET_ASM_RELAX_SPEC "%{m4*:-isa=sh4-up}"
 #endif

+/* Define which ISA type to pass to the assembler.
+   For SH4 we pass SH4A to allow using some instructions that are available
+   on some SH4 variants, but officially are part of the SH4A ISA.  */
 #define SH_ASM_SPEC \
  "%(subtarget_asm_endian_spec) %{mrelax:-relax %(subtarget_asm_relax_spec)} \
 %(subtarget_asm_isa_spec) %(subtarget_asm_spec) \
+%{m1:--isa=sh} \
+%{m2:--isa=sh2} \
+%{m2e:--isa=sh2e} \
+%{m3:--isa=sh3} \
+%{m3e:--isa=sh3e} \
+%{m4:--isa=sh4a} \
+%{m4-single:--isa=sh4a} \
+%{m4-single-only:--isa=sh4a} \
+%{m4-nofpu:--isa=sh4a-nofpu} \
+%{m4a:--isa=sh4a} \
+%{m4a-single:--isa=sh4a} \
+%{m4a-single-only:--isa=sh4a} \
+%{m4a-nofpu:--isa=sh4a-nofpu} \
 %{m2a:--isa=sh2a} \
 %{m2a-single:--isa=sh2a} \
 %{m2a-single-only:--isa=sh2a} \

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[olegendo at gcc dot gnu.org]

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

--- Comment #12 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #10)
> 
> FAIL: gcc.dg/torture/vshuf-v16hi.c  -O2  (test for excess errors)
> Excess errors:
> /usr/local/sh-elf/bin/ld: internal error: merge of architecture 'sh3e' with
> architecture 'sh2a-nofpu' produced unknown architecture
> /usr/local/sh-elf/bin/ld: /tmp/ccRPtSqs.o: uses instructions which are
> incompatible with instructions used in previous modules
> /usr/local/sh-elf/bin/ld: failed to merge target specific data of file
> /tmp/ccRPtSqs.o

Below is the reduced test case that fails when compiled with -O2 -m4 -mb
when the r,r constraints are allowed in the "*movhi" pattern and the
"*mov<mode>_reg_reg" pattern is disabled.  So the comment above the
"*mov<mode>_reg_reg" pattern is still correct.

typedef unsigned short V __attribute__((vector_size(32)));
typedef V VI;

extern void abort (void);
V a, b, c, d;

 __attribute__((noinline, noclone)) void
test_14 (void)
{
  VI mask = { 25, 5, 17, 1, 9, 15, 21, 7, 28, 2, 18, 13, 30, 14, 10, 4 };
  int i;
  c = __builtin_shuffle (a, mask);
  d = __builtin_shuffle (a, b, mask);
  __asm ("" : : "r" (&c), "r" (&d) : "memory");

  for (i = 0; i < 16; ++i)
    if (c[i] != a[mask[i] & (16 - 1)])
      abort ();
    else if ((mask[i] & 16))
    {
      if (d[i] != b[mask[i] & (16 - 1)])
        abort ();
    }
    else if (d[i] != a[mask[i] & (16 - 1)])
      abort ();
}

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[olegendo at gcc dot gnu.org]

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

--- Comment #13 from Oleg Endo <olegendo at gcc dot gnu.org> ---
BTW, using the "m" constraint for QImode and HImode in inline asm is dangerous.
 It's very easy to create wrong code with it.  For example:

struct test_struct
{
  unsigned short a, b, c, d;
};

void test (struct test_struct* s, unsigned short* result)
{
  unsigned short r;

  __asm__ __volatile__ (
    "mov.w    %1,%0"
    : "=&r" (r)
    : "m" (s->b)
    : "memory");

  *result = r;
}

will compile to:

_test:
! 39 "sh_tmp.cpp" 1
        mov.w   @(2,r4),r1    ! Invalid mov.w for non-SH2A.
! 0 "" 2
        rts
        mov.w   r1,@r5


I think in order to avoid surprises and simplify debugging of such code
something like the patch in comment #11 should be added to trunk.

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[chrbr at gcc dot gnu.org]

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

--- Comment #15 from chrbr at gcc dot gnu.org ---
Thanks Oleg, I'll give it a try for 4.8.3

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[kkojima at gcc dot gnu.org]

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

--- Comment #16 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
With the patch, no new failures on trunk for sh4-unknown-linux-gnu
assumed the patch for PR59243.

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[chrbr at gcc dot gnu.org]

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

--- Comment #17 from chrbr at gcc dot gnu.org ---
> Although not fully tested yet, could you guys please have a look at it?
> Christian, does it fix your Linux build problems, or are there still more /
> new ones?

the 2.6.32 kernel build is fixed. thanks

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[olegendo at gcc dot gnu.org]

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

--- Comment #18 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Tue Nov 26 11:48:16 2013
New Revision: 205390

URL: http://gcc.gnu.org/viewcvs?rev=205390&root=gcc&view=rev
Log:
    PR target/58314
    PR target/50751
    * config/sh/sh.c (max_mov_insn_displacement, disp_addr_displacement):
    Prefix function names with 'sh_'.  Make them non-static.
    * config/sh/sh-protos.h (sh_disp_addr_displacement,
    sh_max_mov_insn_displacement): Add declarations.
    * config/sh/constraints.md (Q): Reject QImode.
    (Sdd): Use match_code "mem".
    (Snd): Fix erroneous matching of non-memory operands.
    * config/sh/predicates.md (short_displacement_mem_operand): New
    predicate.
    (general_movsrc_operand): Disallow PC relative QImode loads.
    * config/sh/sh.md (*mov<mode>_reg_reg): Remove it.
    (*movqi, *movhi): Merge both insns into...
    (*mov<mode>): ... this new insn.  Replace generic 'm' constraints with
    'Snd' and 'Sdd' constraints.  Calculate insn length dynamically based
    on the operand types.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/constraints.md
    trunk/gcc/config/sh/predicates.md
    trunk/gcc/config/sh/sh-protos.h
    trunk/gcc/config/sh/sh.c
    trunk/gcc/config/sh/sh.md

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[olegendo at gcc dot gnu.org]

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

--- Comment #19 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Fri Dec  6 19:34:23 2013
New Revision: 205759

URL: http://gcc.gnu.org/viewcvs?rev=205759&root=gcc&view=rev
Log:
    Backport from mainline
    2013-11-26  Oleg Endo  <olegendo@gcc.gnu.org>

    PR target/58314
    PR target/50751
    * config/sh/sh.c (max_mov_insn_displacement, disp_addr_displacement):
    Prefix function names with 'sh_'.  Make them non-static.
    * config/sh/sh-protos.h (sh_disp_addr_displacement,
    sh_max_mov_insn_displacement): Add declarations.
    * config/sh/constraints.md (Q): Reject QImode.
    (Sdd): Use match_code "mem".
    (Snd): Fix erroneous matching of non-memory operands.
    * config/sh/predicates.md (short_displacement_mem_operand): New
    predicate.
    (general_movsrc_operand): Disallow PC relative QImode loads.
    * config/sh/sh.md (*mov<mode>_reg_reg): Remove it.
    (*movqi, *movhi): Merge both insns into...
    (*mov<mode>): ... this new insn.  Replace generic 'm' constraints with
    'Snd' and 'Sdd' constraints.  Calculate insn length dynamically based
    on the operand types.


Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/sh/constraints.md
    branches/gcc-4_8-branch/gcc/config/sh/predicates.md
    branches/gcc-4_8-branch/gcc/config/sh/sh-protos.h
    branches/gcc-4_8-branch/gcc/config/sh/sh.c
    branches/gcc-4_8-branch/gcc/config/sh/sh.md

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[olegendo at gcc dot gnu.org]

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

Oleg Endo <olegendo at gcc dot gnu.org> changed:

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

--- Comment #20 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Fixed on trunk (4.9) and 4.8.

[Bug target/58314] SH4 error: 'asm' operand requires impossible reload

[olegendo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58314

--- Comment #21 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Mon Dec 22 18:53:44 2014
New Revision: 219030

URL: https://gcc.gnu.org/viewcvs?rev=219030&root=gcc&view=rev
Log:
gcc/testsuite/
    PR target/58314
    * gcc.target/sh/torture/pr58314-2.c: New.
    * gcc.target/sh/torture/pr58314.c: Don't set -Os option.

Added:
    trunk/gcc/testsuite/gcc.target/sh/torture/pr58314-2.c
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/sh/torture/pr58314.c