[Bug target/94538] New: [9/10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[Bug target/94538] New: [9/10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[zsojka at seznam dot cz]

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

            Bug ID: 94538
           Summary: [9/10 Regression] ICE: in
                    extract_constrain_insn_cached, at recog.c:2223 (insn
                    does not satisfy its constraints) with
                    -mcpu=cortex-m23 -mslow-flash-data
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zsojka at seznam dot cz
  Target Milestone: ---
              Host: x86_64-pc-linux-gnu
            Target: armv7a-hardfloat-linux-gnueabi

Created attachment 48247
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48247&action=edit
reduced testcase

Compiler output:
$ armv7a-hardfloat-linux-gnueabi-gcc -mcpu=cortex-m23 -mslow-flash-data
testcase.c
testcase.c: In function 'foo':
testcase.c:9:1: error: insn does not satisfy its constraints:
    9 | }
      | ^
(insn 91 90 17 (set (reg:SI 12 ip [128])
        (plus:SI (reg:SI 12 ip [128])
            (const_int 255 [0xff]))) "testcase.c":8:5 905 {*thumb1_addsi3}
     (expr_list:REG_EQUAL (const_int 4095 [0xfff])
        (nil)))
during RTL pass: shorten
testcase.c:9:1: internal compiler error: in extract_constrain_insn_cached, at
recog.c:2207
0x6f8802 _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
        /repo/gcc-trunk/gcc/rtl-error.c:108
0x6f888f _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
        /repo/gcc-trunk/gcc/rtl-error.c:118
0x6e9a7e extract_constrain_insn_cached(rtx_insn*)
        /repo/gcc-trunk/gcc/recog.c:2207
0x166862f insn_default_length(rtx_insn*)
        /repo/gcc-trunk/gcc/config/arm/arm.md:363
0xcb4c3e shorten_branches(rtx_insn*)
        /repo/gcc-trunk/gcc/final.c:1118
0xcb4d0f rest_of_handle_shorten_branches
        /repo/gcc-trunk/gcc/final.c:4753
0xcb4d0f execute
        /repo/gcc-trunk/gcc/final.c:4782
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

$ armv7a-hardfloat-linux-gnueabi-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-armv7a-hardfloat/bin/armv7a-hardfloat-linux-gnueabi-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r10-7640-20200408212358-gd0cc1b79b39-checking-yes-rtl-df-extra-armv7a-hardfloat/bin/../libexec/gcc/armv7a-hardfloat-linux-gnueabi/10.0.1/lto-wrapper
Target: armv7a-hardfloat-linux-gnueabi
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--with-cloog --with-ppl --with-isl --with-float=hard --with-fpu=vfpv4
--with-arch=armv7-a --with-sysroot=/usr/armv7a-hardfloat-linux-gnueabi
--build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu
--target=armv7a-hardfloat-linux-gnueabi
--with-ld=/usr/bin/armv7a-hardfloat-linux-gnueabi-ld
--with-as=/usr/bin/armv7a-hardfloat-linux-gnueabi-as --disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r10-7640-20200408212358-gd0cc1b79b39-checking-yes-rtl-df-extra-armv7a-hardfloat
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 10.0.1 20200408 (experimental) (GCC)

[Bug target/94538] [9/10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[wilco at gcc dot gnu.org]

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

Wilco <wilco at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
                 CC|                            |wilco at gcc dot gnu.org
   Last reconfirmed|                            |2020-04-09
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |wilco at gcc dot gnu.org

--- Comment #1 from Wilco <wilco at gcc dot gnu.org> ---
Thanks for the concise testcase. -mslow-flash-data disables MOVW generation
which exactly the opposite of what we want. I'm testing a trivial fix.

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[wilco at gcc dot gnu.org]

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

Wilco <wilco at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[9/10 Regression] ICE: in   |[10 Regression] ICE: in
                   |extract_constrain_insn_cach |extract_constrain_insn_cach
                   |ed, at recog.c:2223 (insn   |ed, at recog.c:2223 (insn
                   |does not satisfy its        |does not satisfy its
                   |constraints) with           |constraints) with
                   |-mcpu=cortex-m23            |-mcpu=cortex-m23
                   |-mslow-flash-data           |-mslow-flash-data

--- Comment #2 from Wilco <wilco at gcc dot gnu.org> ---
This was introduced by commit e24f6408d so only in GCC10.

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[zsojka at seznam dot cz]

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

--- Comment #3 from Zdenek Sojka <zsojka at seznam dot cz> ---
(In reply to Wilco from comment #2)
> This was introduced by commit e24f6408d so only in GCC10.

Thank you for checking this!

I am quite sure this fails in gcc-9 as well:

$ armv7a-hardfloat-linux-gnueabi-gcc -mcpu=cortex-m23 -mslow-flash-data
testcase.c
testcase.c: In function 'foo':
testcase.c:9:1: error: insn does not satisfy its constraints:
    9 | }
      | ^
(insn 91 90 17 (set (reg:SI 12 ip [125])
        (plus:SI (reg:SI 12 ip [125])
            (const_int 255 [0xff]))) "testcase.c":8:5 812 {*thumb1_addsi3}
     (expr_list:REG_EQUAL (const_int 4095 [0xfff])
        (nil)))
during RTL pass: shorten
testcase.c:9:1: internal compiler error: in extract_constrain_insn_cached, at
recog.c:2223
0x68bade _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
        /repo/gcc-9-branch/gcc/rtl-error.c:108
0x68bb6b _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
        /repo/gcc-9-branch/gcc/rtl-error.c:118
0x67c6a2 extract_constrain_insn_cached(rtx_insn*)
        /repo/gcc-9-branch/gcc/recog.c:2223
0x14555b3 insn_default_length(rtx_insn*)
        /repo/gcc-9-branch/gcc/config/arm/arm.md:376
0xbc958e shorten_branches(rtx_insn*)
        /repo/gcc-9-branch/gcc/final.c:1119
0xbc965f rest_of_handle_shorten_branches
        /repo/gcc-9-branch/gcc/final.c:4754
0xbc965f execute
        /repo/gcc-9-branch/gcc/final.c:4783
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

$ armv7a-hardfloat-linux-gnueabi-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-9-branch/binary-latest-armv7a-hardfloat/bin/armv7a-hardfloat-linux-gnueabi-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-9-branch/binary-9-branch-20200407210206-g14192f1ed48-checking-yes-rtl-df-extra-armv7a-hardfloat/bin/../libexec/gcc/armv7a-hardfloat-linux-gnueabi/9.3.1/lto-wrapper
Target: armv7a-hardfloat-linux-gnueabi
Configured with: /repo/gcc-9-branch//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--with-cloog --with-ppl --with-isl --with-float=hard --with-fpu=vfpv4
--with-arch=armv7-a --with-sysroot=/usr/armv7a-hardfloat-linux-gnueabi
--build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu
--target=armv7a-hardfloat-linux-gnueabi
--with-ld=/usr/bin/armv7a-hardfloat-linux-gnueabi-ld
--with-as=/usr/bin/armv7a-hardfloat-linux-gnueabi-as --disable-libstdcxx-pch
--prefix=/repo/gcc-9-branch//binary-9-branch-20200407210206-g14192f1ed48-checking-yes-rtl-df-extra-armv7a-hardfloat
Thread model: posix
gcc version 9.3.1 20200407 (GCC) 

Perhaps the offending commit, or part of it, was backported to gcc-9 as well?

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[wilco at gcc dot gnu.org]

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

--- Comment #4 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Zdenek Sojka from comment #3)
> (In reply to Wilco from comment #2)
> > This was introduced by commit e24f6408d so only in GCC10.
> 
> Thank you for checking this!
> 
> I am quite sure this fails in gcc-9 as well:
...
> Perhaps the offending commit, or part of it, was backported to gcc-9 as well?

It's possible it was recently backported and our GCC9 builds don't yet have it.
But that whole patch is badly broken and introduces multiple issues...

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

https://gcc.gnu.org/g:0666767eb4cc864f00ba34d97b9d58f8dc650bba

commit r10-7682-g0666767eb4cc864f00ba34d97b9d58f8dc650bba
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Fri Apr 10 20:55:10 2020 +0100

    coroutines: Revise await expansions [PR94528]

    The expansions for await expressions were specific to particular
    cases, this revises it to be more generic.

    a: Revise co_await statement walkers.

    We want to process the co_awaits one statement at a time.
    We also want to be able to determine the insertion points for
    new bind scopes needed to cater for temporaries that are
    captured by reference and have lifetimes that need extension
    to the end of the full expression.  Likewise, the handling of
    captured references in the evaluation of conditions might
    result in the need to make a frame copy.

    This reorganises the statement walking code to make it easier to
    extend for these purposes.

    b: Factor reference-captured temp code.

    We want to be able to use the code that writes a new bind expr
    with vars (and their initializers) from several places, so split
    that out of the maybe_promote_captured_temps() function into a
    new replace_statement_captures ().  Update some comments.

    c: Generalize await statement expansion.

    This revises the expansion to avoid the need to expand conditionally
    on the tree type.  It resolves PR 94528.

    gcc/cp/ChangeLog:

    2020-04-10  Iain Sandoe  <iain@sandoe.co.uk>

            PR c++/94538
            * coroutines.cc (co_await_expander): Remove.
            (expand_one_await_expression): New.
            (process_one_statement): New.
            (await_statement_expander): New.
            (build_actor_fn): Revise to use per-statement expander.
            (struct susp_frame_data): Reorder and comment.
            (register_awaits): Factor code.
            (replace_statement_captures): New, factored from...
            (maybe_promote_captured_temps):.. here.
            (await_statement_walker): Revise to process per statement.
            (morph_fn_to_coro): Use revised susp_frame_data layout.

    gcc/testsuite/ChangeLog:

    2020-04-10  Iain Sandoe  <iain@sandoe.co.uk>

            PR c++/94538
            * g++.dg/coroutines/pr94528.C: New test.

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

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

--- Comment #6 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to CVS Commits from comment #5)
> The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

sorry about the bad PR number, this commit is unrelated to the PR.

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[wilco at gcc dot gnu.org]

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

--- Comment #7 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Wilco from comment #4)
> (In reply to Zdenek Sojka from comment #3)
> > (In reply to Wilco from comment #2)
> > > This was introduced by commit e24f6408d so only in GCC10.
> > 
> > Thank you for checking this!
> > 
> > I am quite sure this fails in gcc-9 as well:
> ...
> > Perhaps the offending commit, or part of it, was backported to gcc-9 as well?
> 
> It's possible it was recently backported and our GCC9 builds don't yet have
> it. But that whole patch is badly broken and introduces multiple issues...

Adding Christophe. I'm thinking the best approach right now is to revert given
-mpure-code doesn't work at all on Thumb-1 targets - it still emits literal
pools, switch tables etc. That's not pure code!

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[clyon at gcc dot gnu.org]

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

--- Comment #8 from Christophe Lyon <clyon at gcc dot gnu.org> ---

> Adding Christophe. I'm thinking the best approach right now is to revert
> given -mpure-code doesn't work at all on Thumb-1 targets - it still emits
> literal pools, switch tables etc. That's not pure code!

Do you have testcases that show these failures?

I did check some of the problematic testcases in the GCC testsuite when I
committed that patch. Did I miss some of them?

Can you point me to offending testcases and compiler options so that I can
reproduce them?

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[zsojka at seznam dot cz]

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

--- Comment #9 from Zdenek Sojka <zsojka at seznam dot cz> ---
Please don't take this PR serious; it might not be a valid use-case in
real-world application.

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[wilco at gcc dot gnu.org]

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

--- Comment #10 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Christophe Lyon from comment #8)
> > Adding Christophe. I'm thinking the best approach right now is to revert
> > given -mpure-code doesn't work at all on Thumb-1 targets - it still emits
> > literal pools, switch tables etc. That's not pure code!
> 
> Do you have testcases that show these failures?
> 
> I did check some of the problematic testcases in the GCC testsuite when I
> committed that patch. Did I miss some of them?
> 
> Can you point me to offending testcases and compiler options so that I can
> reproduce them?

For example:

int x;
int f1 (void) { return x; }

with eg. -O2 -mcpu=cortex-m0 -mpure-code I get:

        movs    r3, #:upper8_15:#.LC1
        lsls    r3, #8
        adds    r3, #:upper0_7:#.LC1
        lsls    r3, #8
        adds    r3, #:lower8_15:#.LC1
        lsls    r3, #8
        adds    r3, #:lower0_7:#.LC1
        @ sp needed
        ldr     r3, [r3]
        ldr     r0, [r3, #40]
        bx      lr

That's an extra indirection through a literal... There should only be one ldr
to read x.

Big switch tables are produced for any Thumb-1 core, however I would expect
Cortex-m0/m23 versions to look almost identical to the Cortex-m3 one, and use a
sequence of comparisons instead of tables.

int f2 (int x, int y)
{
  switch (x)
  {
    case 0: return y + 0;
    case 1: return y + 1;
    case 2: return y + 2;
    case 3: return y + 3;
    case 4: return y + 4;
    case 5: return y + 5;
  }
  return y;
}

Immediate generation for common cases seems to be screwed up:

int f3 (void) { return 0x11000000; }

-O2 -mcpu=cortex-m0 -mpure-code:

        movs    r0, #17
        lsls    r0, r0, #8
        lsls    r0, r0, #8
        lsls    r0, r0, #8
        bx      lr

This also regressed Cortex-m23 which previously generated:

        movs    r0, #136
        lsls    r0, r0, #21
        bx      lr

Similar regressions happen with other immediates:

int f3 (void) { return 0x12345678; }

-O2 -mcpu=cortex-m23 -mpure-code:

        movs    r0, #86
        lsls    r0, r0, #8
        adds    r0, r0, #120
        movt    r0, 4660
        bx      lr

Previously it was:

        movw    r0, #22136
        movt    r0, 4660
        bx      lr

Also relocations with a small offset should be handled within the relocation.
I'd expect this to never generate an extra addition, let alone an extra literal
pool entry:

int arr[10];
int *f4 (void) { return &arr[1]; }

-O2 -mcpu=cortex-m3 -mpure-code generates the expected:

        movw    r0, #:lower16:.LANCHOR0+4
        movt    r0, #:upper16:.LANCHOR0+4
        bx      lr

-O2 -mcpu=cortex-m23 -mpure-code generates this:

        movw    r0, #:lower16:.LANCHOR0
        movt    r0, #:upper16:.LANCHOR0
        adds    r0, r0, #4
        bx      lr

And cortex-m0 again inserts an extra literal load:

        movs    r3, #:upper8_15:#.LC0
        lsls    r3, #8
        adds    r3, #:upper0_7:#.LC0
        lsls    r3, #8
        adds    r3, #:lower8_15:#.LC0
        lsls    r3, #8
        adds    r3, #:lower0_7:#.LC0
        ldr     r0, [r3]
        adds    r0, r0, #4
        bx      lr

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[clyon at gcc dot gnu.org]

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

--- Comment #11 from Christophe Lyon <clyon at gcc dot gnu.org> ---
(In reply to Wilco from comment #10)
> 
> For example:
> 
> int x;
> int f1 (void) { return x; }
> 
> with eg. -O2 -mcpu=cortex-m0 -mpure-code I get:
> 
>         movs    r3, #:upper8_15:#.LC1
>         lsls    r3, #8
>         adds    r3, #:upper0_7:#.LC1
>         lsls    r3, #8
>         adds    r3, #:lower8_15:#.LC1
>         lsls    r3, #8
>         adds    r3, #:lower0_7:#.LC1
>         @ sp needed
>         ldr     r3, [r3]
>         ldr     r0, [r3, #40]
>         bx      lr
> 
> That's an extra indirection through a literal... There should only be
> one ldr to read x.

Right, but the code is functional. I mentioned that problem when I submitted
the patch. I thought it was good to provide functionality and improve the
generated code later.
I wrote: "I haven't found yet how to make code for cortex-m0 apply upper/lower
relocations to "p" instead of .LC2. The current code looks functional, but
could be improved."

> 
> Big switch tables are produced for any Thumb-1 core, however I would expect
> Cortex-m0/m23 versions to look almost identical to the Cortex-m3 one, and
> use a sequence of comparisons instead of tables.
> 
> int f2 (int x, int y)
> {
>   switch (x)
>   {
>     case 0: return y + 0;
>     case 1: return y + 1;
>     case 2: return y + 2;
>     case 3: return y + 3;
>     case 4: return y + 4;
>     case 5: return y + 5;
>   }
>   return y;
> }
> 

I believe this is expected: as I wrote in my commit message
"CASE_VECTOR_PC_RELATIVE is now false with -mpure-code, to avoid generating
invalid assembly code with differences from symbols from two different sections
(the difference cannot be computed by the assembler)."

Maybe there's a possibility to tune this to detect cases where we can do
better?


> Immediate generation for common cases seems to be screwed up:
> 
> int f3 (void) { return 0x11000000; }
> 
> -O2 -mcpu=cortex-m0 -mpure-code:
> 
>         movs    r0, #17
>         lsls    r0, r0, #8
>         lsls    r0, r0, #8
>         lsls    r0, r0, #8
>         bx      lr

This is not optimal, but functional, right?


> This also regressed Cortex-m23 which previously generated:
> 
>         movs    r0, #136
>         lsls    r0, r0, #21
>         bx      lr
> Similar regressions happen with other immediates:
> 
> int f3 (void) { return 0x12345678; }
> 
> -O2 -mcpu=cortex-m23 -mpure-code:
> 
>         movs    r0, #86
>         lsls    r0, r0, #8
>         adds    r0, r0, #120
>         movt    r0, 4660
>         bx      lr
> 
> Previously it was:
> 
>         movw    r0, #22136
>         movt    r0, 4660
>         bx      lr
OK, I'll check how to fix that.


> Also relocations with a small offset should be handled within the
> relocation. I'd expect this to never generate an extra addition, let alone
> an extra literal pool entry:
> 
> int arr[10];
> int *f4 (void) { return &arr[1]; }
> 
> -O2 -mcpu=cortex-m3 -mpure-code generates the expected:
> 
>         movw    r0, #:lower16:.LANCHOR0+4
>         movt    r0, #:upper16:.LANCHOR0+4
>         bx      lr
> 
> -O2 -mcpu=cortex-m23 -mpure-code generates this:
> 
>         movw    r0, #:lower16:.LANCHOR0
>         movt    r0, #:upper16:.LANCHOR0
>         adds    r0, r0, #4
>         bx      lr

For cortex-m23, I get the same code with and without -mpure-code.

> 
> And cortex-m0 again inserts an extra literal load:
> 
>         movs    r3, #:upper8_15:#.LC0
>         lsls    r3, #8
>         adds    r3, #:upper0_7:#.LC0
>         lsls    r3, #8
>         adds    r3, #:lower8_15:#.LC0
>         lsls    r3, #8
>         adds    r3, #:lower0_7:#.LC0
>         ldr     r0, [r3]
>         adds    r0, r0, #4
>         bx      lr
Yes, same problem as in f1()


So I think -mpure-code for v6m is not broken, but yes the generated code can be
improved. So this may not be relevant to this PR?

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[clyon at gcc dot gnu.org]

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

--- Comment #12 from Christophe Lyon <clyon at gcc dot gnu.org> ---
I've posted a patch to fix the regression for your f3() examples:
https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543993.html

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[wilco at gcc dot gnu.org]

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

--- Comment #13 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Christophe Lyon from comment #12)
> I've posted a patch to fix the regression for your f3() examples:
> https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543993.html

Yes that improves some of the examples, but I still see regressions on m0 for
eg. -1 or 511. The splitter must have the right priority or it will continue to
cause regressions. That means placing it around line 790 after the existing
movsi splitters.

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[wilco at gcc dot gnu.org]

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

--- Comment #14 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Christophe Lyon from comment #11)
> (In reply to Wilco from comment #10)

> Right, but the code is functional.

It doesn't avoid the literal load from flash which is exactly what pure-code
and slow-flash-data is all about.

That brings me to another question, why does -mslow-flash-data give an error
when used with Cortex-M0? If the goal was to make things more orthogonal then
surely we should not give an error and allow both options on all M-class cores.

> I believe this is expected: as I wrote in my commit message
> "CASE_VECTOR_PC_RELATIVE is now false with -mpure-code, to avoid generating
> invalid assembly code with differences from symbols from two different
> sections (the difference cannot be computed by the assembler)."
> 
> Maybe there's a possibility to tune this to detect cases where we can do
> better?

The best option is to do the same as Cortex-M3: just switch off branch tables
altogether and fall back to compare+branch. That completely avoids loading data
from flash and is always smaller than emitting 32-bit tables.


> > Also relocations with a small offset should be handled within the
> > relocation. I'd expect this to never generate an extra addition, let alone
> > an extra literal pool entry:
> > 
> > int arr[10];
> > int *f4 (void) { return &arr[1]; }
> > 
> > -O2 -mcpu=cortex-m3 -mpure-code generates the expected:
> > 
> >         movw    r0, #:lower16:.LANCHOR0+4
> >         movt    r0, #:upper16:.LANCHOR0+4
> >         bx      lr
> > 
> > -O2 -mcpu=cortex-m23 -mpure-code generates this:
> > 
> >         movw    r0, #:lower16:.LANCHOR0
> >         movt    r0, #:upper16:.LANCHOR0
> >         adds    r0, r0, #4
> >         bx      lr
> 
> For cortex-m23, I get the same code with and without -mpure-code.

GCC9 emits something different for Cortex-M0 and Cortex-M23 so this was changed
by the patch somehow even when -mpure-code is not enabled. So it is a
regression from what we used to generate.

Similarly I would not expect pure-code to change the decision of whether to
emit immediates as part of the relocation as long as they are within range. The
existing implementation for Cortex-M3 does this correctly.

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[clyon at gcc dot gnu.org]

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

--- Comment #15 from Christophe Lyon <clyon at gcc dot gnu.org> ---
(In reply to Wilco from comment #14)
> (In reply to Christophe Lyon from comment #11)
> > (In reply to Wilco from comment #10)
> 
> > Right, but the code is functional.
> 
> It doesn't avoid the literal load from flash which is exactly what pure-code
> and slow-flash-data is all about.

For f1 on M0, I can see:
        .section        .rodata.cst4,"aM",%progbits,4
        .align  2
.LC0:
        .word   .LANCHOR0
        .section .text,"0x20000006",%progbits
[...]
f1:
        movs    r3, #:upper8_15:#.LC0
        lsls    r3, #8
        adds    r3, #:upper0_7:#.LC0
        lsls    r3, #8
        adds    r3, #:lower8_15:#.LC0
        lsls    r3, #8
        adds    r3, #:lower0_7:#.LC0
        ldr     r3, [r3]        @ 6     [c=10 l=2]  *thumb1_movsi_insn/8
        ldr     r0, [r3]        @ 7     [c=10 l=2]  *thumb1_movsi_insn/8
        bx      lr
[...]
        .bss
        .align  2
        .set    .LANCHOR0,. + 0
        .type   x, %object
        .size   x, 4
x:
        .space  4

So the 1st load is from .rodata.cst4 and the 2nd load is from bss, both of
which do not have the purecode bit set (unlike .text). Isn't that OK?

[Bug target/94538] [10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[wilco at gcc dot gnu.org]

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

--- Comment #16 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Christophe Lyon from comment #15)
> (In reply to Wilco from comment #14)
> > (In reply to Christophe Lyon from comment #11)
> > > (In reply to Wilco from comment #10)
> > 
> > > Right, but the code is functional.
> > 
> > It doesn't avoid the literal load from flash which is exactly what pure-code
> > and slow-flash-data is all about.
> 
> For f1 on M0, I can see:
>         .section        .rodata.cst4,"aM",%progbits,4
>         .align  2
> .LC0:
>         .word   .LANCHOR0
>         .section .text,"0x20000006",%progbits
> [...]
> f1:
>         movs    r3, #:upper8_15:#.LC0
>         lsls    r3, #8
>         adds    r3, #:upper0_7:#.LC0
>         lsls    r3, #8
>         adds    r3, #:lower8_15:#.LC0
>         lsls    r3, #8
>         adds    r3, #:lower0_7:#.LC0
>         ldr     r3, [r3]        @ 6     [c=10 l=2]  *thumb1_movsi_insn/8
>         ldr     r0, [r3]        @ 7     [c=10 l=2]  *thumb1_movsi_insn/8
>         bx      lr
> [...]
>         .bss
>         .align  2
>         .set    .LANCHOR0,. + 0
>         .type   x, %object
>         .size   x, 4
> x:
>         .space  4
> 
> So the 1st load is from .rodata.cst4 and the 2nd load is from bss, both of
> which do not have the purecode bit set (unlike .text). Isn't that OK?

No, it will create a lot of complaints and support queries due to the obvious
regressions. It goes against the definition of pure-code and slow-flash-data
which is to remove the literal loads. And given the sequence is already
inefficient, we should do everything to remove the indirection which increases
the codesize overhead by 75%...

Another aspect that needs to be checked is that GCC correctly spills addresses
and complex constants instead of rematerializing them. This is basic minimal
quality that one expects for a feature like this.

[Bug target/94538] [9/10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[10 Regression] ICE: in     |[9/10 Regression] ICE: in
                   |extract_constrain_insn_cach |extract_constrain_insn_cach
                   |ed, at recog.c:2223 (insn   |ed, at recog.c:2223 (insn
                   |does not satisfy its        |does not satisfy its
                   |constraints) with           |constraints) with
                   |-mcpu=cortex-m23            |-mcpu=cortex-m23
                   |-mslow-flash-data           |-mslow-flash-data
   Target Milestone|---                         |9.4

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> ---
Can somebody check whether 9.3.0 is affected and fill out known-to-work
appropriately?

[Bug target/94538] [9/10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[clyon at gcc dot gnu.org]

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

Christophe Lyon <clyon at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |9.2.0

--- Comment #18 from Christophe Lyon <clyon at gcc dot gnu.org> ---
I've just checked that the testcase passes with 9.2.0

[Bug target/94538] [9/10 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[wilco at gcc dot gnu.org]

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

--- Comment #19 from Wilco <wilco at gcc dot gnu.org> ---
Yes I have a GCC9.3 build now, this fails too.

[Bug target/94538] [9/10/11 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[cvs-commit at gcc dot gnu.org]

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

--- Comment #20 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Christophe Lyon <clyon@gcc.gnu.org>:

https://gcc.gnu.org/g:259d072067997ab8f55afcf735c91b6740fd0425

commit r11-2818-g259d072067997ab8f55afcf735c91b6740fd0425
Author: Christophe Lyon <christophe.lyon@linaro.org>
Date:   Wed Aug 19 09:02:21 2020 +0000

    arm: Fix -mpure-code support/-mslow-flash-data for armv8-m.base [PR94538]

    armv8-m.base (cortex-m23) has the movt instruction, so we need to
    disable the define_split to generate a constant in this case,
    otherwise we get incorrect insn constraints as described in PR94538.

    We also need to fix the pure-code alternative for thumb1_movsi_insn
    because the assembler complains with instructions like
    movs r0, #:upper8_15:1234
    (Internal error in md_apply_fix)
    We now generate movs r0, 4 instead.

    2020-08-24  Christophe Lyon  <christophe.lyon@linaro.org>

            PR target/94538
            gcc/
            * config/arm/thumb1.md: Disable set-constant splitter when
            TARGET_HAVE_MOVT.
            (thumb1_movsi_insn): Fix -mpure-code
            alternative.

            PR target/94538
            gcc/testsuite/
            * gcc.target/arm/pure-code/pr94538-1.c: New test.
            * gcc.target/arm/pure-code/pr94538-2.c: New test.

[Bug target/94538] [9/10/11 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[clyon at gcc dot gnu.org]

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

--- Comment #21 from Christophe Lyon <clyon at gcc dot gnu.org> ---
I filed PR96767, PR96768, PR96769, PR96770 to track the enhancements discussed
here.

The ICE is now fixed in trunk.

[Bug target/94538] [9/10/11 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[cvs-commit at gcc dot gnu.org]

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

--- Comment #22 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Christophe Lyon
<clyon@gcc.gnu.org>:

https://gcc.gnu.org/g:967a5f8e585a5d499870903849c0fa06c326547a

commit r9-8835-g967a5f8e585a5d499870903849c0fa06c326547a
Author: Christophe Lyon <christophe.lyon@linaro.org>
Date:   Wed Aug 19 09:02:21 2020 +0000

    arm: Fix -mpure-code support/-mslow-flash-data for armv8-m.base [PR94538]

    armv8-m.base (cortex-m23) has the movt instruction, so we need to
    disable the define_split to generate a constant in this case,
    otherwise we get incorrect insn constraints as described in PR94538.

    We also need to fix the pure-code alternative for thumb1_movsi_insn
    because the assembler complains with instructions like
    movs r0, #:upper8_15:1234
    (Internal error in md_apply_fix)
    We now generate movs r0, 4 instead.

    2020-08-24  Christophe Lyon  <christophe.lyon@linaro.org>

            PR target/94538
            gcc/
            * config/arm/thumb1.md: Disable set-constant splitter when
            TARGET_HAVE_MOVT.
            (thumb1_movsi_insn): Fix -mpure-code
            alternative.

            PR target/94538
            gcc/testsuite/
            * gcc.target/arm/pure-code/pr94538-1.c: New test.
            * gcc.target/arm/pure-code/pr94538-2.c: New test.

    (cherry picked from commit 259d072067997ab8f55afcf735c91b6740fd0425)

[Bug target/94538] [9/10/11 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[cvs-commit at gcc dot gnu.org]

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

--- Comment #23 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Christophe Lyon
<clyon@gcc.gnu.org>:

https://gcc.gnu.org/g:f8bc014aa47a1b2056e89e50b0ce86ba64d93f3b

commit r10-8678-gf8bc014aa47a1b2056e89e50b0ce86ba64d93f3b
Author: Christophe Lyon <christophe.lyon@linaro.org>
Date:   Wed Aug 19 09:02:21 2020 +0000

    arm: Fix -mpure-code support/-mslow-flash-data for armv8-m.base [PR94538]

    armv8-m.base (cortex-m23) has the movt instruction, so we need to
    disable the define_split to generate a constant in this case,
    otherwise we get incorrect insn constraints as described in PR94538.

    We also need to fix the pure-code alternative for thumb1_movsi_insn
    because the assembler complains with instructions like
    movs r0, #:upper8_15:1234
    (Internal error in md_apply_fix)
    We now generate movs r0, 4 instead.

    2020-08-24  Christophe Lyon  <christophe.lyon@linaro.org>

            PR target/94538
            gcc/
            * config/arm/thumb1.md: Disable set-constant splitter when
            TARGET_HAVE_MOVT.
            (thumb1_movsi_insn): Fix -mpure-code
            alternative.

            PR target/94538
            gcc/testsuite/
            * gcc.target/arm/pure-code/pr94538-1.c: New test.
            * gcc.target/arm/pure-code/pr94538-2.c: New test.

    (cherry picked from commit 259d072067997ab8f55afcf735c91b6740fd0425)

[Bug target/94538] [9/10/11 Regression] ICE: in extract_constrain_insn_cached, at recog.c:2223 (insn does not satisfy its constraints) with -mcpu=cortex-m23 -mslow-flash-data

[clyon at gcc dot gnu.org]

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

Christophe Lyon <clyon at gcc dot gnu.org> changed:

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

--- Comment #24 from Christophe Lyon <clyon at gcc dot gnu.org> ---
Backported to gcc-9 and gcc-10, thus considered fixed.

Improvements tracked in the new PRs.