On Wed, Jun 18, 2025 at 2:30 PM Hongtao Liu <[email protected]> wrote:
>
> On Mon, May 26, 2025 at 2:30 PM H.J. Lu <[email protected]> wrote:
> >
> > On Sun, May 25, 2025 at 7:02 PM H.J. Lu <[email protected]> wrote:
> > >
> > > On Sun, May 25, 2025 at 8:12 AM H.J. Lu <[email protected]> wrote:
> > > >
> > > > On Sun, May 25, 2025 at 7:47 AM H.J. Lu <[email protected]> wrote:
> > > > >
> > > > > commit ef26c151c14a87177d46fd3d725e7f82e040e89f
> > > > > Author: Roger Sayle <[email protected]>
> > > > > Date: Thu Dec 23 12:33:07 2021 +0000
> > > > >
> > > > > x86: PR target/103773: Fix wrong-code with -Oz from pop to memory.
> > > > >
> > > > > transformed "mov $0,mem" to the shorter and "$0,mem" for -Oz. But
> > > > >
> > > > > (define_insn "*mov<mode>_and"
> > > > > [(set (match_operand:SWI248 0 "memory_operand" "=m")
> > > > > (match_operand:SWI248 1 "const0_operand"))
> > > > > (clobber (reg:CC FLAGS_REG))]
> > > > > "reload_completed"
> > > > > "and{<imodesuffix>}\t{%1, %0|%0, %1}"
> > > > > [(set_attr "type" "alu1")
> > > > > (set_attr "mode" "<MODE>")
> > > > > (set_attr "length_immediate" "1")])
> > > > >
> > > > > isn't guarded for -Oz. As a result, "and $0,mem" is generated without
> > > > > -Oz. Enable *mov<mode>_and only for -Oz.
> > > > >
> > > > > gcc/
> > > > >
> > > > > PR target/120427
> > > > > * config/i386/i386.md (*mov<mode>_and): Enable only for -Oz.
> > > > >
> > > > > gcc/testsuite/
> > > > >
> > > > > PR target/120427
> > > > > * gcc.target/i386/pr120427.c: New test.
> > > > >
> > > > > OK for master?
> > > > >
> > > >
> > > > "mov $-1,mem" has the same issue. Here is the updated patch to also
> > > > enable "or $-1,mem" only for -Oz.
> > > >
> > > > OK for master?
> > >
> > > It doesn't work since "*mov<mode>_or" was extended from load. Here is
> > > the v2 patch:
> > >
> > > 1. Add "*mov<mode>_or_store" for "or $-1,mem".
> > > 2. Rename "*mov<mode>_or" to "*mov<mode>_or_load", replacing
> > > nonimmediate_operand with register_operand.
> > > 3. Enable "*mov<mode>_and" and "*mov<mode>_or_store" only for -Oz.
> > >
> > > Tested on x86-64.
> >
> > Here is the v3 patch. Change "*mov<mode>_or" to define_insn_and_split
> > and split it to "mov $-1,mem" if not -Oz. Don't transform "mov $-1,reg" to
> > "push $-1; pop reg" for -Oz since it should be transformed to "or $-1,reg".
> >
> > Tested on x86-64. OK for master?
>
> >@@ -2442,18 +2442,24 @@ (define_insn "*mov<mode>_and"
> > [(set (match_operand:SWI248 0 "memory_operand" "=m")
> > (match_operand:SWI248 1 "const0_operand"))
> > (clobber (reg:CC FLAGS_REG))]
> >- "reload_completed"
> >+ "reload_completed
> >+ && optimize_insn_for_size_p () && optimize_size > 1"
>
> Can we also change *mov<mode>_and to define_insn_and_split similar as
> you did for *mov<mode>_or?
Fixed.
> >--- a/gcc/testsuite/gcc.target/i386/cold-attribute-4.c
> >+++ b/gcc/testsuite/gcc.target/i386/cold-attribute-4.c
> >@@ -1,5 +1,5 @@
> > /* { dg-do compile } */
> >-/* { dg-options "-O2" } */
> >+/* { dg-options "-Oz" } */
> > #include <string.h>
> >
> >int
> >__attribute__ ((cold))
> >t(int c)
> >{
> > return -1;
> >}
>
> There's a COLD attribute for the function, so I assume the function
> will be compiled with Oz internally and no need to change the option?
This change is needed since __attribute__ ((cold)) doesn't imply -Oz.
Will send the v4 patch later.
Thanks.
--
H.J.
