[Bug target/114232] New: [14 regression] ICE when building rr-5.7.0 with LTO on x86

[Bug target/114232] New: [14 regression] ICE when building rr-5.7.0 with LTO on x86

[sjames at gcc dot gnu.org]

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

            Bug ID: 114232
           Summary: [14 regression] ICE when building rr-5.7.0 with LTO on
                    x86
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sjames at gcc dot gnu.org
  Target Milestone: ---

Created attachment 57608
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57608&action=edit
RecordSession.cc.ii.xz

Hit this when building rr-5.7.0 with LTO on x86.

```
$ cat list.txt

RecordSession.cc.ii
Task.cc.ii
TraceStream.cc.ii

$ g++ -O3 -pipe -march=i686 -mfpmath=sse -msse -msse2 -fno-vect-cost-model
-rdynamic -flto=auto @list.txt
/var/tmp/portage/dev-util/rr-5.7.0/work/rr-5.7.0/src/TraceStream.cc: In member
function ‘close’:
/var/tmp/portage/dev-util/rr-5.7.0/work/rr-5.7.0/src/TraceStream.cc:1467:1:
error: unrecognizable insn:
 1467 | }
      | ^
(insn 160 159 161 26 (parallel [
            (set (reg:V2QI 250 [ vect_patt_207.470_183 ])
                (minus:V2QI (reg:V2QI 251)
                    (reg:V2QI 249 [ vect__4.468_451 ])))
            (clobber (reg:CC 17 flags))
        ])
"/var/tmp/portage/dev-util/rr-5.7.0/work/rr-5.7.0/src/TraceStream.cc":254:16 -1
     (nil))
during RTL pass: vregs
/var/tmp/portage/dev-util/rr-5.7.0/work/rr-5.7.0/src/TraceStream.cc:1467:1:
internal compiler error: in extract_insn, at recog.cc:2812
0x5799263a _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
       
/usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/rtl-error.cc:108
0x579927e8 _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
       
/usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/rtl-error.cc:116
0x56eadade extract_insn(rtx_insn*)
        /usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/recog.cc:2812
0x58ac1379 instantiate_virtual_regs_in_insn
       
/usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/function.cc:1611
0x58ac1379 instantiate_virtual_regs
       
/usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/function.cc:1994
0x58ac1379 execute
       
/usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/function.cc:2041
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
Please include the complete backtrace with any bug report.
See <https://bugs.gentoo.org/> for instructions.
make: *** [/tmp/ccCI1g9e.mk:17: /tmp/ccZVEvZf.ltrans5.ltrans.o] Error 1
lto-wrapper: fatal error: make returned 2 exit status
compilation terminated.
/usr/lib/gcc/i686-pc-linux-gnu/14/../../../../i686-pc-linux-gnu/bin/ld: error:
lto-wrapper failed
collect2: error: ld returned 1 exit status
```

```
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/i686-pc-linux-gnu/14/lto-wrapper
Target: i686-pc-linux-gnu
Configured with:
/var/tmp/portage/sys-devel/gcc-14.0.9999/work/gcc-14.0.9999/configure
--host=i686-pc-linux-gnu --build=i686-pc-linux-gnu --prefix=/usr
--bindir=/usr/i686-pc-linux-gnu/gcc-bin/14
--includedir=/usr/lib/gcc/i686-pc-linux-gnu/14/include
--datadir=/usr/share/gcc-data/i686-pc-linux-gnu/14
--mandir=/usr/share/gcc-data/i686-pc-linux-gnu/14/man
--infodir=/usr/share/gcc-data/i686-pc-linux-gnu/14/info
--with-gxx-include-dir=/usr/lib/gcc/i686-pc-linux-gnu/14/include/g++-v14
--disable-silent-rules --disable-dependency-tracking
--with-python-dir=/share/gcc-data/i686-pc-linux-gnu/14/python
--enable-languages=c,c++,fortran --enable-obsolete --enable-secureplt
--disable-werror --with-system-zlib --enable-nls --without-included-gettext
--disable-libunwind-exceptions --enable-checking=yes,extra,rtl,df
--with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo 14.0.9999 p,
commit c8305c9bdf09abe3e2f89783fe62f2e4049468fa' --with-gcc-major-version-only
--enable-libstdcxx-time --enable-lto --disable-libstdcxx-pch --enable-shared
--enable-threads=posix --enable-__cxa_atexit --enable-clocale=gnu
--disable-multilib --disable-fixed-point --with-arch=i686 --enable-targets=all
--enable-libgomp --disable-libssp --disable-libada --disable-cet
--disable-systemtap --enable-valgrind-annotations --disable-vtable-verify
--disable-libvtv --with-zstd --without-isl --enable-default-pie
--enable-host-pie --disable-host-bind-now --enable-default-ssp
--disable-fixincludes --with-build-config='bootstrap-O3 bootstrap-lto'
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 14.0.1 20240304 (experimental)
a89c5df317d1de74871e2a05c36aed9cbbb21f42 (Gentoo 14.0.9999 p, commit
c8305c9bdf09abe3e2f89783fe62f2e4049468fa)
```

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[sjames at gcc dot gnu.org]

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

--- Comment #1 from Sam James <sjames at gcc dot gnu.org> ---
Created attachment 57609
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57609&action=edit
Task.cc.ii.xz

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[sjames at gcc dot gnu.org]

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

--- Comment #2 from Sam James <sjames at gcc dot gnu.org> ---
Created attachment 57610
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57610&action=edit
TraceStream.cc.ii.xz

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[sjames at gcc dot gnu.org]

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

--- Comment #3 from Sam James <sjames at gcc dot gnu.org> ---
I am reducing it now but it's slow going.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[ubizjak at gmail dot com]

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

--- Comment #4 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Sam James from comment #0)

> (insn 160 159 161 26 (parallel [
>             (set (reg:V2QI 250 [ vect_patt_207.470_183 ])
>                 (minus:V2QI (reg:V2QI 251)
>                     (reg:V2QI 249 [ vect__4.468_451 ])))
>             (clobber (reg:CC 17 flags))
>         ])

This is the definition of the offending pattern in mmx.md:

(define_insn "<insn>v2qi3"
  [(set (match_operand:V2QI 0 "register_operand" "=?Q,x,Yw")
        (plusminus:V2QI
          (match_operand:V2QI 1 "register_operand" "<comm>0,0,Yw")
          (match_operand:V2QI 2 "register_operand" "Q,x,Yw")))
   (clobber (reg:CC FLAGS_REG))]
  "!TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun)"
  "#"
  [(set_attr "isa" "*,sse2_noavx,avx")
   (set_attr "type" "multi,sseadd,sseadd")
   (set_attr "mode" "QI,TI,TI")])

where -march=i686 (aka pentiumpro) implies TARGET_PARTIAL_REG_STALL, so it
should be disabled unless optimizing for size. I wonder if
optimize_function_for_size_p is stable during the LTO compilation, but we have
plenty of usages like the above throughout x86  .md files and there were no
problems reported.

Another possibility is that the instruction RTX is emitted without checking of
the pattern condition, the testcase is compiled with -O3, so
optimize_function_for_size_p should also be false.

I don't see anything wrong with the above pattern. The failure also happens
very early into the RTL part of the compilation (vregs pass is the first pass
that tries to recognize the pattern), so my bet is on middle-end emitting insn
pattern without checking for pattern availability.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[ubizjak at gmail dot com]

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

--- Comment #5 from Uroš Bizjak <ubizjak at gmail dot com> ---
Huh, it looks that optimize_function_for_size_p (cfun) is not stable during
LTO?!

Using:

--cut here--
diff --git a/gcc/config/i386/mmx.md b/gcc/config/i386/mmx.md
index 2856ae6ffef..80114494b0b 100644
--- a/gcc/config/i386/mmx.md
+++ b/gcc/config/i386/mmx.md
@@ -2975,7 +2975,7 @@ (define_insn "<insn>v2qi3"
          (match_operand:V2QI 1 "register_operand" "<comm>0,0,Yw")
          (match_operand:V2QI 2 "register_operand" "Q,x,Yw")))
    (clobber (reg:CC FLAGS_REG))]
-  "!TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun)"
+  "!TARGET_PARTIAL_REG_STALL"
   "#"
   [(set_attr "isa" "*,sse2_noavx,avx")
    (set_attr "type" "multi,sseadd,sseadd")
@@ -2987,7 +2987,7 @@ (define_split
          (match_operand:V2QI 1 "general_reg_operand")
          (match_operand:V2QI 2 "general_reg_operand")))
    (clobber (reg:CC FLAGS_REG))]
-  "(!TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun))
+  "!TARGET_PARTIAL_REG_STALL
    && reload_completed"
   [(parallel
      [(set (strict_low_part (match_dup 0))
@@ -3021,7 +3021,7 @@ (define_split
          (match_operand:V2QI 1 "sse_reg_operand")
          (match_operand:V2QI 2 "sse_reg_operand")))
    (clobber (reg:CC FLAGS_REG))]
-  "(!TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun))
+  "!TARGET_PARTIAL_REG_STALL
    && TARGET_SSE2 && reload_completed"
   [(set (match_dup 0)
         (plusminus:V16QI (match_dup 1) (match_dup 2)))]
--cut here--

So, removing optimize-size bypass successfully compiles the testcase.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
It's possibly on a cold path (yes, optimize_function_for_size_p should be
stable).  Note though that optimize_function_for_size_p might in theory
change between vectorization and RTL expansion, so maybe optab queries
in the vectorizer are broken by this if we ever re-check that condition
after optab initialization.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
                 CC|                            |hubicka at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think the question is more whether it's stable between optab queries the
vectorizer or veclower does and RTL expansion.  There whether it's LTO or not
shouldn't play a role (it might for the actual testcase of course).

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
> grep optimize_ insn-flags.h | wc -l
14

so it's not very many standard patterns that would be affected.  I'd say
using these kind of flags on standard patterns is at least fragile?

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[ubizjak at gmail dot com]

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

--- Comment #9 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Richard Biener from comment #8)
> > grep optimize_ insn-flags.h | wc -l
> 14
> 
> so it's not very many standard patterns that would be affected.  I'd say
> using these kind of flags on standard patterns is at least fragile?

You are right, only recently added neg, add, sub, ashl, lshr, ashr V2QImode
standard patterns have optimize_function_for_size_p in their condition.

Any suggestion on how to fix them?

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[ubizjak at gmail dot com]

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

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2024-03-05
           Assignee|unassigned at gcc dot gnu.org      |ubizjak at gmail dot com
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

--- Comment #10 from Uroš Bizjak <ubizjak at gmail dot com> ---
Created attachment 57612
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57612&action=edit
Prototype patch

Let's try this approach.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Uroš Bizjak from comment #10)
> Created attachment 57612 [details]
> Prototype patch
> 
> Let's try this approach.

Yeah, I guess !TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun)
is best elided (or also avoid the pattern when optimizing for size).

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Still, it would be nice to understand what changed optimize_function_for_size_p
(cfun)
after IPA.  Is something adjusting node->count or node->frequency?
Otherwise it should just depend on the optimize_size flag which should not
change...

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[ubizjak at gmail dot com]

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

--- Comment #13 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Jakub Jelinek from comment #12)
> Still, it would be nice to understand what changed
> optimize_function_for_size_p (cfun)
> after IPA.  Is something adjusting node->count or node->frequency?
> Otherwise it should just depend on the optimize_size flag which should not
> change...

The target-dependent issue is with insn patterns we split to. These are enabled
with "!TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun)", as
they are intended to be combined from movstrict insn pattern (which can FAIL).

So, the condition for V2QI insn is now either !TARGET_PARTIAL_REG_STALL or
TARGET_SSE2 (where we disable alternative 0 for TARGET_PARTIAL_REG_STALL
targets, but we still emit SSE instruction).

Please note that while -march=i686 enables TARGET_PARTIAL_REG_STALL, it enables
SSE2, so the proposed solution won't have much impact. Also, V2QImode is not
much used, so I guess the proposed solution is the right compromise.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[rguenther at suse dot de]

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

--- Comment #14 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 5 Mar 2024, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114232
> 
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org
> 
> --- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Still, it would be nice to understand what changed optimize_function_for_size_p
> (cfun)
> after IPA.  Is something adjusting node->count or node->frequency?
> Otherwise it should just depend on the optimize_size flag which should not
> change...

Maybe we share the TREE optimization node (it gets re-materialized
during LTO streaming) between hot and cold functions and the "first"
one getting in set_cfun will overwrite TREE_OPTIMIZATION_OPTABS in
init_tree_optimization_optabs (though it seems to overwrite things).

In any case I think the tree node sharing only looks at options, not
at function frequency so having TREE_OPTIMIZATION_OPTABS part of
the optimization node looks a bit fragile in the light of sharing.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[jakub at gcc dot gnu.org]

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Yeah, indeed, the optabs enable flags are cached in the optimization node, so
it is ok
to check the optimization flags in there, or target flags as well, but
optimize_function_for_*_p is not, because it depends also on
node->count/node->frequency of the current function but multiple functions can
have the same optimization node/target node combo.
Seems i386 is the only backend which does something like that.
I believe this has been fixed in the backend in the past already, see e.g.
PR92791
While
      if (opts != optimization_default_node)
        {
          init_tree_optimization_optabs (opts);
          if (TREE_OPTIMIZATION_OPTABS (opts))
            this_fn_optabs = (struct target_optabs *)
              TREE_OPTIMIZATION_OPTABS (opts);
        }
in invoke_set_current_function_hook maybe (hopefully) makes stuff work right
for the different options, that certainly doesn't account for
node->function/node->count.

Seems various backends use e.g. optimize_size or !optimize_size or optimize > 0
etc. in
insn-flags.h, so perhaps change optimize_function_for_size_p (cfun) to
optimize_size?

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[ubizjak at gmail dot com]

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

--- Comment #16 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Jakub Jelinek from comment #15)

> Seems various backends use e.g. optimize_size or !optimize_size or optimize
> > 0 etc. in
> insn-flags.h, so perhaps change optimize_function_for_size_p (cfun) to
> optimize_size?

Won't this cause a mismatch with insn patterns we split to? As said in Comment
#13, these have insn predicates that include optimize_function_for_size_p.

Re: [Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[Jan Hubicka]

Looking at the prototype patch, why need to change also the splitters?

My original goal was to use splitters to expand to faster code sequences
while having patterns necessary for both variants.  This makes it
possible to use optimize_insn_for_size/speed and make decisions using BB
profile, since we will not ICE if the hotness of BB changes later.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[jakub at gcc dot gnu.org]

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

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Either change those too, or the splitter needs some variant what to do if there
is a mismatch.
Though, optimize_size implies optimize_function_for_size_p (cfun), so if a
named pattern uses && optimize_size and the insn it splits into uses
optimize_function_for_size_p (cfun), it shouldn't fail.  The other direction is
not always true, optimize_function_for_size_p (cfun) can be true just because
the function
is cold, not just because it is -Os.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[hubicka at ucw dot cz]

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

--- Comment #18 from Jan Hubicka <hubicka at ucw dot cz> ---
optimize_function_for_size_p is not really affected by LTO or non-LTO. 
It does take into account node->count and node->frequency, which is
updated during IPA, so it may change between early opts and late opts.

I do not think we change it between vectorizer and RTL (or during RTL)
and we can add this to a verifier if it is practical to rely on it (I
can see that vectorizer makes instruction selection choices).

But the problem here is more that optab initializations happens only at
the optimization_node changes and not if we switch from hot function to
cold?

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[ubizjak at gmail dot com]

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

--- Comment #19 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Jan Hubicka from comment #18)
> But the problem here is more that optab initializations happens only at
> the optimization_node changes and not if we switch from hot function to
> cold?

I think solving optab init problem is a better solution than the target patch
from comment #10. Using optimize_function_for_size_p in named pattern predicate
would avoid using the non-optimal pattern also in cold functions, and would be
preferrable to using optimize_size.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[jakub at gcc dot gnu.org]

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

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Uroš Bizjak from comment #19)
> (In reply to Jan Hubicka from comment #18)
> > But the problem here is more that optab initializations happens only at
> > the optimization_node changes and not if we switch from hot function to
> > cold?
> 
> I think solving optab init problem is a better solution than the target
> patch from comment #10. Using optimize_function_for_size_p in named pattern
> predicate would avoid using the non-optimal pattern also in cold functions,
> and would be preferrable to using optimize_size.

It would be very costly IMHO, because on every set_cfun we'd need to compute
optimize_function_for_size_p for both the old and new function and find out
where to attach the additional optab tables. set_cfun can be called millions of
times.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[hubicka at ucw dot cz]

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

--- Comment #21 from Jan Hubicka <hubicka at ucw dot cz> ---
Looking at the prototype patch, why need to change also the splitters?

My original goal was to use splitters to expand to faster code sequences
while having patterns necessary for both variants.  This makes it
possible to use optimize_insn_for_size/speed and make decisions using BB
profile, since we will not ICE if the hotness of BB changes later.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[rguenther at suse dot de]

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

--- Comment #22 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 5 Mar 2024, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114232
> 
> --- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Either change those too, or the splitter needs some variant what to do if there
> is a mismatch.
> Though, optimize_size implies optimize_function_for_size_p (cfun), so if a
> named pattern uses && optimize_size and the insn it splits into uses
> optimize_function_for_size_p (cfun), it shouldn't fail.  The other direction is
> not always true, optimize_function_for_size_p (cfun) can be true just because
> the function
> is cold, not just because it is -Os.

I think optimize_function_for_size_p (cfun) isn't always true if
optimize_size is since it looks at the function-specific setting
of that flag, so you'd have to use opt_for_fn (cfun, optimize_size).

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[ubizjak at gmail dot com]

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

--- Comment #23 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Jan Hubicka from comment #21)
> Looking at the prototype patch, why need to change also the splitters?

Purely for implementation reasons, we check for general resp. SSE register in
the operand predicates, so there is no need to use additional insn constraints.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[jakub at gcc dot gnu.org]

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

--- Comment #24 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #22)
> I think optimize_function_for_size_p (cfun) isn't always true if
> optimize_size is since it looks at the function-specific setting
> of that flag, so you'd have to use opt_for_fn (cfun, optimize_size).

I think the insn-flags.h macros and actual insn conditions are checked when the
corresponding function is current, and at that point opt_for_fn (cfun,
optimize_size) should be the same as optimize_size.  Even the set_cfun hook
first sets the option and then tries to initialize optab and compares it
against the global one.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[ubizjak at gmail dot com]

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

--- Comment #25 from Uroš Bizjak <ubizjak at gmail dot com> ---
Created attachment 57614
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57614&action=edit
Proposed patch

Proposed patch that changes optimize_function_for_size_p (cfun) to
optimize_size.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[hubicka at ucw dot cz]

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

--- Comment #26 from Jan Hubicka <hubicka at ucw dot cz> ---
> I think optimize_function_for_size_p (cfun) isn't always true if
> optimize_size is since it looks at the function-specific setting
> of that flag, so you'd have to use opt_for_fn (cfun, optimize_size).

When we initialize obtabs, we do it for a givn optimization_node
setting, so opt_for_fn (cfun, optimize_size) should be equal to
optimize_size.

Honza

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[ubizjak at gmail dot com]

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

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #57614|0                           |1
        is obsolete|                            |

--- Comment #27 from Uroš Bizjak <ubizjak at gmail dot com> ---
Created attachment 57626
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57626&action=edit
Proposed v2 patch

New version in testing:
  - uses optimize_size to stabilize optab discovery
  - explicitly enables insn pattern for TARGET_SSE2

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[cvs-commit at gcc dot gnu.org]

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

--- Comment #28 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Uros Bizjak <uros@gcc.gnu.org>:

https://gcc.gnu.org/g:74e8cc28eda9b1d75588fcd4017a735911b9d2b4

commit r14-9346-g74e8cc28eda9b1d75588fcd4017a735911b9d2b4
Author: Uros Bizjak <ubizjak@gmail.com>
Date:   Wed Mar 6 20:53:50 2024 +0100

    i386: Fix and improve insn constraint for V2QI arithmetic/shift insns

    optimize_function_for_size_p predicate is not stable during optab
selection,
    because it also depends on node->count/node->frequency of the current
function,
    which are updated during IPA, so they may change between early opts and
    late opts.  Use optimize_size instead - optimize_size implies
    optimize_function_for_size_p (cfun), so if a named pattern uses
    "&& optimize_size" and the insn it splits into uses
    optimize_function_for_size_p (cfun), it shouldn't fail.

            PR target/114232

    gcc/ChangeLog:

            * config/i386/mmx.md (negv2qi2): Enable for optimize_size instead
            of optimize_function_for_size_p.  Explictily enable for
TARGET_SSE2.
            (negv2qi SSE reg splitter): Enable for TARGET_SSE2 only.
            (<plusminus:insn>v2qi3): Enable for optimize_size instead
            of optimize_function_for_size_p.  Explictily enable for
TARGET_SSE2.
            (<plusminus:insn>v2qi SSE reg splitter): Enable for TARGET_SSE2
only.
            (<any_shift:insn>v2qi3): Enable for optimize_size instead
            of optimize_function_for_size_p.

[Bug target/114232] [14 regression] ICE when building rr-5.7.0 with LTO on x86

[ubizjak at gmail dot com]

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

Uroš Bizjak <ubizjak at gmail dot com> changed:

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

--- Comment #29 from Uroš Bizjak <ubizjak at gmail dot com> ---
Fixed.