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