Looks good to me.  Sorry for any inconvenience.
Cheers,
Roger

> -----Original Message-----
> From: Hongtao Liu <crazy...@gmail.com>
> Sent: 19 June 2025 08:01
> To: H.J. Lu <hjl.to...@gmail.com>
> Cc: GCC Patches <gcc-patches@gcc.gnu.org>; Uros Bizjak <ubiz...@gmail.com>;
> Hongtao Liu <hongtao....@intel.com>; Roger Sayle
> <ro...@nextmovesoftware.com>
> Subject: Re: [PATCH v4] x86: Enable *mov<mode>_(and|or) only for -Oz
> 
> On Wed, Jun 18, 2025 at 6:38 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> >
> > commit ef26c151c14a87177d46fd3d725e7f82e040e89f
> > Author: Roger Sayle <ro...@nextmovesoftware.com>
> > Date:   Thu Dec 23 12:33:07 2021 +0000
> >
> >     x86: PR target/103773: Fix wrong-code with -Oz from pop to memory.
> >
> > added "*mov<mode>_and" and extended "*mov<mode>_or" to transform
> "mov
> > $0,mem" to the shorter "and $0,mem" and "mov $-1,mem" to the shorter
> > "or $-1,mem" for -Oz.  But the new pattern:
> >
> > (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")])
> >
> > and the extended pattern:
> >
> > (define_insn "*mov<mode>_or"
> >   [(set (match_operand:SWI248 0 "nonimmediate_operand" "=rm")
> >     (match_operand:SWI248 1 "constm1_operand"))
> >    (clobber (reg:CC FLAGS_REG))]
> >   "reload_completed"
> >   "or{<imodesuffix>}\t{%1, %0|%0, %1}"
> >   [(set_attr "type" "alu1")
> >    (set_attr "mode" "<MODE>")
> >    (set_attr "length_immediate" "1")])
> >
> > aren't guarded for -Oz.  As a result, "and $0,mem" and "or $-1,mem"
> > are generated without -Oz.
> >
> > 1. Change *mov<mode>_and" to define_insn_and_split and split it to
> > "mov $0,mem" if not -Oz.
> > 2. Change "*mov<mode>_or" to define_insn_and_split and split it to
> > "mov $-1,mem" if not -Oz.
> > 3. Don't transform "mov $-1,reg" to "push $-1; pop reg" for -Oz since
> > it should be transformed to "or $-1,reg".
> 
> Ok for me, but please hold for a few days in case Uros or Roger has any
> comments.
> 
> >
> > gcc/
> >
> > PR target/120427
> > * config/i386/i386.md (*mov<mode>_and): Changed to
> > define_insn_and_split.  Split it to "mov $0,mem" if not -Oz.
> > (*mov<mode>_or): Changed to define_insn_and_split.  Split it to "mov
> > $-1,mem" if not -Oz.
> > (peephole2): Don't transform "mov $-1,reg" to "push $-1; pop reg"
> > for -Oz since it will be transformed to "or $-1,reg".
> >
> > gcc/testsuite/
> >
> > PR target/120427
> > * gcc.target/i386/cold-attribute-4.c: Compile with -Oz.
> > * gcc.target/i386/pr120427-1.c: New test.
> > * gcc.target/i386/pr120427-2.c: Likewise.
> > * gcc.target/i386/pr120427-3.c: Likewise.
> > * gcc.target/i386/pr120427-4.c: Likewise.
> >
> >
> > --
> > H.J.
> 
> 
> 
> --
> BR,
> Hongtao

Reply via email to