On Fri, Oct 9, 2026 at 10:31 AM Naveen <[email protected]> wrote: > > >> HImode conditional move writes only the low 16 bits if the > >> condition is true; the upper bits are preserved either way. If the > >> condition is false, nothing changes. > > Thanks very much for pointing this out. You are right that an HImode > conditional move is a read/modify/write operation on the low 16 bits > with the upper bits preserved. I updated the pattern to use > strict_low_part with a read/write (+r,r) destination and added tests > covering both the optimized and fallback cases. > > The x86 backend misses an optimization for a zero-extended HImode > unsigned minimum with a constant. > > For example: > > unsigned f (unsigned short x) > { > return (x > 0xff ? 0xff : x) & 0xff; > } > > The backend currently emits a wider conditional move followed by an > unnecessary zero extension. Add a splitter that initializes the SImode > destination with the constant and performs the minimum using an HImode > conditional move. > > An HImode conditional move updates only the low 16 bits and preserves > the upper bits. Model this with strict_low_part and a read/write > destination. The split is deferred until after reload because LRA cannot > reload strict_low_part in/out operands with matched inputs (PR82524). > > Avoid the transformation on targets with partial-register dependencies, > except when optimizing for size. Keep the explicit FLAGS_REG use so the > post-reload SWI12 conditional-move splitter does not widen the HImode > conditional move.
No. The conversion to SImode is there for a reason, and if you have to trick the compiler to avoid the conversion, it hints that it is not wanted at all. HImode operations actually *insert* HImode value into SImode register, so they have to take care of extended register dependency chain. The compiler is actively avoiding HImode instructions (unless they are actual inserts) and your patch goes in the opposite direction. You are trading one zero-extension for a whole bunch of hidden optimization issues. And we had many issues with cmove in the past, so unless there is clear performance benefit, I would skip proposed optimization. Uros. > Regression tested on x86_64-linux with no new failures. > > gcc/ChangeLog: > > PR target/94543 > * config/i386/i386.md (*movhicc_noc_flags_use): New insn > pattern. > (*zero_extend_uminhisi2): New insn-and-split pattern. > > gcc/testsuite/ChangeLog: > > PR target/94543 > * gcc.target/i386/pr94543.c: New test. > * gcc.target/i386/pr94543-1.c: New test. > > Signed-off-by: Naveen <[email protected]> > --- > gcc/config/i386/i386.md | 51 +++++++++++++++++++++++ > gcc/testsuite/gcc.target/i386/pr94543-1.c | 11 +++++ > gcc/testsuite/gcc.target/i386/pr94543.c | 12 ++++++ > 3 files changed, 74 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/i386/pr94543-1.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr94543.c > > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > index d5b90291fc9..b6cf859748e 100644 > --- a/gcc/config/i386/i386.md > +++ b/gcc/config/i386/i386.md > @@ -27513,6 +27513,26 @@ > (set (match_dup 0) > (neg:SWI (ltu:SWI (reg:CCC FLAGS_REG) (const_int 0))))]) > > +;; Keep the explicit flags use in the pattern below. It prevents the > +;; post-reload SWI12 conditional-move splitter from widening a HImode move > +;; that is used as the low part of an SImode value. > +(define_insn "*movhicc_noc_flags_use" > + [(set (strict_low_part > + (match_operand:HI 0 "register_operand" "+r,r")) > + (if_then_else:HI (match_operator 1 "ix86_comparison_operator" > + [(reg FLAGS_REG) (const_int 0)]) > + (match_operand:HI 2 "nonimmediate_operand" "rm,0") > + (match_operand:HI 3 "nonimmediate_operand" "0,rm"))) > + (use (reg:CC FLAGS_REG))] > + "TARGET_CMOVE && reload_completed > + && (!TARGET_PARTIAL_REG_DEPENDENCY || optimize_function_for_size_p (cfun)) > + && !(MEM_P (operands[2]) && MEM_P (operands[3]))" > + "@ > + cmov%O2%C1\t{%2, %0|%0, %2} > + cmov%O2%c1\t{%3, %0|%0, %3}" > + [(set_attr "type" "icmov") > + (set_attr "mode" "HI")]) > + > (define_insn "*mov<mode>cc_noc" > [(set (match_operand:SWI248 0 "register_operand" "=r,r,r,r,r") > (if_then_else:SWI248 (match_operator 1 "ix86_comparison_operator" > @@ -28098,6 +28118,37 @@ > > ;; min/max patterns > > +;; A zero-extended unsigned HImode min can select directly into the low > +;; part of the SImode destination after initializing the destination with > +;; the constant. This avoids a separate zero extension after the cmov. > +;; Keep the split after reload because LRA cannot reload strict_low_part > +;; in-out operands with matched inputs (PR82524). > +(define_insn_and_split "*zero_extend_uminhisi2" > + [(set (match_operand:SI 0 "register_operand" "=&r") > + (zero_extend:SI > + (umin:HI (match_operand:HI 1 "register_operand") > + (match_operand:HI 2 "const_int_operand"))))] > + "TARGET_64BIT && TARGET_CMOVE && !TARGET_PARTIAL_REG_STALL > + && (!TARGET_PARTIAL_REG_DEPENDENCY || optimize_function_for_size_p (cfun)) > + && INTVAL (operands[2]) >= 0 > + && (unsigned HOST_WIDE_INT) INTVAL (operands[2]) > + <= GET_MODE_MASK (HImode)" > + "#" > + "&& reload_completed" > + [(set (match_dup 0) (match_dup 2)) > + (set (reg:CC FLAGS_REG) > + (compare:CC (match_dup 1) (match_dup 3))) > + (parallel > + [(set (strict_low_part (match_dup 4)) > + (if_then_else:HI (leu (reg:CC FLAGS_REG) (const_int 0)) > + (match_dup 1) > + (match_dup 3))) > + (use (reg:CC FLAGS_REG))])] > +{ > + operands[3] = lowpart_subreg (HImode, operands[0], SImode); > + operands[4] = operands[3]; > +}) > + > (define_code_attr maxmin_rel > [(smax "GE") (smin "LE") (umax "GEU") (umin "LEU")]) > > diff --git a/gcc/testsuite/gcc.target/i386/pr94543-1.c > b/gcc/testsuite/gcc.target/i386/pr94543-1.c > new file mode 100644 > index 00000000000..cce67d64f0f > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr94543-1.c > @@ -0,0 +1,11 @@ > +/* PR target/94543 */ > +/* { dg-do compile { target { ! ia32 } } } */ > +/* { dg-options "-O2 -march=x86-64 -mtune-ctrl=partial_reg_dependency" } */ > +/* { dg-final { scan-assembler-not "cmov\[a-z\]+\[ \t\]+%\[a-z0-9\]+, %ax" } > } */ > +/* { dg-final { scan-assembler "movzwl" } } */ > + > +unsigned > +f (unsigned short x) > +{ > + return (x > 0xff ? 0xff : x) & 0xff; > +} > diff --git a/gcc/testsuite/gcc.target/i386/pr94543.c > b/gcc/testsuite/gcc.target/i386/pr94543.c > new file mode 100644 > index 00000000000..9c31f0199fc > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr94543.c > @@ -0,0 +1,12 @@ > +/* PR target/94543 */ > +/* { dg-do compile { target { ! ia32 } } } */ > +/* { dg-options "-O2 -march=x86-64 -mtune-ctrl=^partial_reg_dependency" } */ > +/* { dg-final { scan-assembler "movl\[ \t\]+\\\$255, %eax" } } */ > +/* { dg-final { scan-assembler "cmovbe\[ \t\]+%di, %ax" } } */ > +/* { dg-final { scan-assembler-not "movzwl" } } */ > + > +unsigned > +f (unsigned short x) > +{ > + return (x > 0xff ? 0xff : x) & 0xff; > +} > -- > 2.34.1 >
