* RE: [x86 PATCH] PR target/110551: Fix reg allocation for widening multiplications.
@ 2023-10-18 14:29 Roger Sayle
2023-10-30 6:09 ` Jiang, Haochen
0 siblings, 1 reply; 2+ messages in thread
From: Roger Sayle @ 2023-10-18 14:29 UTC (permalink / raw)
To: gcc-patches; +Cc: 'Uros Bizjak', tobias.burnus
Many thanks to Tobias Burnus for pointing out the mistake/typo in the PR
number.
This fix is for PR 110551, not PR 110511. I'll update the ChangeLog and
filename
of the new testcase, if approved.
Sorry for any inconvenience/confusion.
Cheers,
Roger
--
> -----Original Message-----
> From: Roger Sayle <roger@nextmovesoftware.com>
> Sent: 17 October 2023 20:06
> To: 'gcc-patches@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> Cc: 'Uros Bizjak' <ubizjak@gmail.com>
> Subject: [x86 PATCH] PR target/110511: Fix reg allocation for widening
> multiplications.
>
>
> This patch contains clean-ups of the widening multiplication patterns in
i386.md,
> and provides variants of the existing highpart multiplication
> peephole2 transformations (that tidy up register allocation after reload),
and
> thereby fixes PR target/110511, which is a superfluous move instruction.
>
> For the new test case, compiled on x86_64 with -O2.
>
> Before:
> mulx64: movabsq $-7046029254386353131, %rcx
> movq %rcx, %rax
> mulq %rdi
> xorq %rdx, %rax
> ret
>
> After:
> mulx64: movabsq $-7046029254386353131, %rax
> mulq %rdi
> xorq %rdx, %rax
> ret
>
> The clean-ups are (i) that operand 1 is consistently made register_operand
and
> operand 2 becomes nonimmediate_operand, so that predicates match the
> constraints, (ii) the representation of the BMI2 mulx instruction is
updated to use
> the new umul_highpart RTX, and (iii) because operands
> 0 and 1 have different modes in widening multiplications, "a" is a more
> appropriate constraint than "0" (which avoids spills/reloads containing
SUBREGs).
> The new peephole2 transformations are based upon those at around line 9951
of
> i386.md, that begins with the comment ;; Highpart multiplication
peephole2s to
> tweak register allocation.
> ;; mov imm,%rdx; mov %rdi,%rax; imulq %rdx -> mov imm,%rax; imulq %rdi
>
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and
> make -k check, both with and without --target_board=unix{-m32} with no new
> failures. Ok for mainline?
>
>
> 2023-10-17 Roger Sayle <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
> PR target/110511
> * config/i386/i386.md (<u>mul<mode><dwi>3): Make operands 1 and
> 2 take "regiser_operand" and "nonimmediate_operand" respectively.
> (<u>mulqihi3): Likewise.
> (*bmi2_umul<mode><dwi>3_1): Operand 2 needs to be register_operand
> matching the %d constraint. Use umul_highpart RTX to represent
> the highpart multiplication.
> (*umul<mode><dwi>3_1): Operand 2 should use regiser_operand
> predicate, and "a" rather than "0" as operands 0 and 2 have
> different modes.
> (define_split): For mul to mulx conversion, use the new
> umul_highpart RTX representation.
> (*mul<mode><dwi>3_1): Operand 1 should be register_operand
> and the constraint %a as operands 0 and 1 have different modes.
> (*<u>mulqihi3_1): Operand 1 should be register_operand matching
> the constraint %0.
> (define_peephole2): Providing widening multiplication variants
> of the peephole2s that tweak highpart multiplication register
> allocation.
>
> gcc/testsuite/ChangeLog
> PR target/110511
> * gcc.target/i386/pr110511.c: New test case.
>
>
> Thanks in advance,
> Roger
^ permalink raw reply [flat|nested] 2+ messages in thread
* RE: [x86 PATCH] PR target/110551: Fix reg allocation for widening multiplications.
2023-10-18 14:29 [x86 PATCH] PR target/110551: Fix reg allocation for widening multiplications Roger Sayle
@ 2023-10-30 6:09 ` Jiang, Haochen
0 siblings, 0 replies; 2+ messages in thread
From: Jiang, Haochen @ 2023-10-30 6:09 UTC (permalink / raw)
To: Roger Sayle, gcc-patches; +Cc: 'Uros Bizjak', tobias.burnus
Hi Roger,
It seems that your patch caused some regression on x86_64:
https://gcc.gnu.org/pipermail/gcc-regression/2023-October/078390.html
https://gcc.gnu.org/pipermail/gcc-regression/2023-October/078391.html
Could you help verify that?
A simple reproducer under build folder will be:
make check RUNTESTFLAGS="conformance.exp=std/time/year_month_day/io.cc --target_board='unix{-m64\ -march=cascadelake,-m32\ -march=cascadelake,-m32,-m64}'"
Thx,
Haochen
> -----Original Message-----
> From: Roger Sayle <roger@nextmovesoftware.com>
> Sent: Wednesday, October 18, 2023 10:30 PM
> To: gcc-patches@gcc.gnu.org
> Cc: 'Uros Bizjak' <ubizjak@gmail.com>; tobias.burnus@siemens.com
> Subject: RE: [x86 PATCH] PR target/110551: Fix reg allocation for widening
> multiplications.
>
>
> Many thanks to Tobias Burnus for pointing out the mistake/typo in the PR
> number.
> This fix is for PR 110551, not PR 110511. I'll update the ChangeLog and
> filename
> of the new testcase, if approved.
>
> Sorry for any inconvenience/confusion.
> Cheers,
> Roger
> --
>
> > -----Original Message-----
> > From: Roger Sayle <roger@nextmovesoftware.com>
> > Sent: 17 October 2023 20:06
> > To: 'gcc-patches@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> > Cc: 'Uros Bizjak' <ubizjak@gmail.com>
> > Subject: [x86 PATCH] PR target/110511: Fix reg allocation for widening
> > multiplications.
> >
> >
> > This patch contains clean-ups of the widening multiplication patterns in
> i386.md,
> > and provides variants of the existing highpart multiplication
> > peephole2 transformations (that tidy up register allocation after reload),
> and
> > thereby fixes PR target/110511, which is a superfluous move instruction.
> >
> > For the new test case, compiled on x86_64 with -O2.
> >
> > Before:
> > mulx64: movabsq $-7046029254386353131, %rcx
> > movq %rcx, %rax
> > mulq %rdi
> > xorq %rdx, %rax
> > ret
> >
> > After:
> > mulx64: movabsq $-7046029254386353131, %rax
> > mulq %rdi
> > xorq %rdx, %rax
> > ret
> >
> > The clean-ups are (i) that operand 1 is consistently made register_operand
> and
> > operand 2 becomes nonimmediate_operand, so that predicates match the
> > constraints, (ii) the representation of the BMI2 mulx instruction is
> updated to use
> > the new umul_highpart RTX, and (iii) because operands
> > 0 and 1 have different modes in widening multiplications, "a" is a more
> > appropriate constraint than "0" (which avoids spills/reloads containing
> SUBREGs).
> > The new peephole2 transformations are based upon those at around line
> 9951
> of
> > i386.md, that begins with the comment ;; Highpart multiplication
> peephole2s to
> > tweak register allocation.
> > ;; mov imm,%rdx; mov %rdi,%rax; imulq %rdx -> mov imm,%rax; imulq %rdi
> >
> >
> > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and
> > make -k check, both with and without --target_board=unix{-m32} with no
> new
> > failures. Ok for mainline?
> >
> >
> > 2023-10-17 Roger Sayle <roger@nextmovesoftware.com>
> >
> > gcc/ChangeLog
> > PR target/110511
> > * config/i386/i386.md (<u>mul<mode><dwi>3): Make operands 1 and
> > 2 take "regiser_operand" and "nonimmediate_operand" respectively.
> > (<u>mulqihi3): Likewise.
> > (*bmi2_umul<mode><dwi>3_1): Operand 2 needs to be
> register_operand
> > matching the %d constraint. Use umul_highpart RTX to represent
> > the highpart multiplication.
> > (*umul<mode><dwi>3_1): Operand 2 should use regiser_operand
> > predicate, and "a" rather than "0" as operands 0 and 2 have
> > different modes.
> > (define_split): For mul to mulx conversion, use the new
> > umul_highpart RTX representation.
> > (*mul<mode><dwi>3_1): Operand 1 should be register_operand
> > and the constraint %a as operands 0 and 1 have different modes.
> > (*<u>mulqihi3_1): Operand 1 should be register_operand matching
> > the constraint %0.
> > (define_peephole2): Providing widening multiplication variants
> > of the peephole2s that tweak highpart multiplication register
> > allocation.
> >
> > gcc/testsuite/ChangeLog
> > PR target/110511
> > * gcc.target/i386/pr110511.c: New test case.
> >
> >
> > Thanks in advance,
> > Roger
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-10-30 6:10 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-18 14:29 [x86 PATCH] PR target/110551: Fix reg allocation for widening multiplications Roger Sayle
2023-10-30 6:09 ` Jiang, Haochen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).