On Tue, Nov 11, 2025 at 5:15 AM Karl Meakin <[email protected]> wrote:
>
>
> On 10/11/2025 23:06, Andrew Pinski wrote:
> > On Mon, Nov 10, 2025 at 10:28 AM Karl Meakin via Sourceware Forge
> > <[email protected]> wrote:
> >> Hi gcc-patches mailing list,
> >> Karl Meakin <[email protected]> has requested that the following
> >> forgejo pull request
> >> be published on the mailing list.
> >>
> >> Created on: 2025-09-30 16:40:31+00:00
> >> Latest update: 2025-11-10 18:26:02+00:00
> >> Changes: 5 changed files, 59 additions, 59 deletions
> >> Head revision: karmea01/gcc-TEST ref km/cmpbr-fix/v1 commit
> >> 6140c1bd023e06f317846402be0485fbdca084e3
> >> Base revision: gcc/gcc-TEST ref trunk commit
> >> 6c56609915f2eb3350a167dfc3e6bb4df3becb42 r16-5059-g6c56609915f2eb
> >> Merge base: 6c56609915f2eb3350a167dfc3e6bb4df3becb42
> >> Full diff url: https://forge.sourceware.org/gcc/gcc-TEST/pulls/88.diff
> >> Discussion: https://forge.sourceware.org/gcc/gcc-TEST/pulls/88
> >> Requested Reviewers:
> >>
> >> Fix an ICE when compiling code that does a movcc with floating point
> >> arguments
> >> with `+cmpbr`. The fix was the same as
> >> bc11cbff9e648fdda2798bfa2d7151d5cd164b87,
> >> so in the second commit I merged the two patterns.
> >>
> >> Testing done:
> >> `make check-gcc` and `make check-target` pass.
> >>
> >> ChangeLog:
> >> * v1: Initial series.
> >> * v2: Move checks from the pattern body into the predicate, and remove
> >> unsued pattern
> >> * v3: Rebase against master
> >> * v4: Add GIMPLE test, `cmpbr-5.c`
> > I thought I approved all of these, I know there was an open question
> > about if I could approve patch 5/5 but I think it is obvious enough
> > that 2 people came up with the same patch to remove the pattern, I can
> > just go ahead approve it.
> >
> > Karl,
> > Do you have git write access? If not then I will push these changes for
> > you.
> >
> > Thanks,
> > Andrew Pinski
> >
> Sorry, I thought a new revision would require a new review.
So GCC has a semi-relax approval process. In this case I approved the
patches with a slight change requested (a testcase being added) so a
new revision of the patch set does not need to be sent out for
approval (though in this case since you don't have write access yet,
it does need to be sent out for someone to apply). Also approval on
each individual patch continues over from the previous patch set if
that patch does not change with the whole set.
> But yes, I will need someone else to push for me, I don't have write access
> yet.
I will apply these either today or tomorrow. I am trying to finish a
different patch set first.
Also are you going to be submitting more patches? If so, we should
think about getting you write access; maybe after the next patch set
we can get you write access.
> Thanks
> >>
> >> Changed files:
> >> - A: gcc/testsuite/gcc.target/aarch64/cmpbr-4.c
> >> - A: gcc/testsuite/gcc.target/aarch64/cmpbr-5.c
> >> - M: gcc/config/aarch64/aarch64.md
> >> - M: gcc/config/aarch64/iterators.md
> >> - M: gcc/config/aarch64/predicates.md
> >>
> >>
> >> Karl Meakin (5):
> >> aarch64: Fix condition accepted by mov<GPF>cc
> >> aarch64: Merge mov<ALLI>cc with mov<GPF>cc
> >> aarch64: Remove redundant checks
> >> aarch64: Add `aarch64_comparison_operator_cc`
> >> aarch64: Remove unused pattern
> >>
> >> gcc/config/aarch64/aarch64.md | 74 +++++-----------------
> >> gcc/config/aarch64/iterators.md | 4 ++
> >> gcc/config/aarch64/predicates.md | 15 +++++
> >> gcc/testsuite/gcc.target/aarch64/cmpbr-4.c | 12 ++++
> >> gcc/testsuite/gcc.target/aarch64/cmpbr-5.c | 13 ++++
> >> 5 files changed, 59 insertions(+), 59 deletions(-)
> >> create mode 100644 gcc/testsuite/gcc.target/aarch64/cmpbr-4.c
> >> create mode 100644 gcc/testsuite/gcc.target/aarch64/cmpbr-5.c
> >>
> >> Range-diff against v3:
> >> 1: e4c13b7ab12 ! 1: 1e9c70b1d93 aarch64: Fix condition accepted by
> >> mov<GPF>cc
> >> @@ Commit message
> >>
> >> Apply the same fix from bc11cbff9e648fdda2798bfa2d7151d5cd164b87
> >> ("aarch64: Fix condition accepted by mov<ALLI>cc") to
> >> `MOV<GPF>cc`.
> >> - Fixes ICEs when compiling code such as `cmpbr-4.c` with `+cmpbr`.
> >> + Fixes ICEs when compiling code such as `cmpbr-4.c` and
> >> `cmpbr-5.c` with `+cmpbr`.
> >>
> >> gcc/ChangeLog:
> >>
> >> @@ Commit message
> >> gcc/testsuite/ChangeLog:
> >>
> >> * gcc.target/aarch64/cmpbr-4.c: New test.
> >> + * gcc.target/aarch64/cmpbr-5.c: New test.
> >>
> >> ## gcc/config/aarch64/aarch64.md ##
> >> @@
> >> @@ gcc/testsuite/gcc.target/aarch64/cmpbr-4.c (new)
> >> +void ConvertRGBToHCL(Quantum red, Quantum green) {
> >> + if (red == MagickMax(red, green)) __builtin_abort();
> >> +}
> >> +
> >> + ## gcc/testsuite/gcc.target/aarch64/cmpbr-5.c (new) ##
> >> +@@
> >> ++/* { dg-do compile } */
> >> ++/* { dg-options "-fgimple -O1" } */
> >> ++#pragma GCC target "+cmpbr"
> >> ++typedef unsigned short us;
> >> ++__GIMPLE double
> >> ++f (us x, us y, double a, double b)
> >> ++{
> >> ++ bool c;
> >> ++ double d;
> >> ++ c = x == y;
> >> ++ d = c ? a : b;
> >> ++ return d;
> >> ++}
> >> 2: 47f37c2eed8 = 2: e56f15f92bf aarch64: Merge mov<ALLI>cc with
> >> mov<GPF>cc
> >> 3: e928669f383 = 3: dd1c190a751 aarch64: Remove redundant checks
> >> 4: 7d8df3a0c82 = 4: 10eab1ed3c8 aarch64: Add
> >> `aarch64_comparison_operator_cc`
> >> 5: d3c842f4f76 = 5: 6140c1bd023 aarch64: Remove unused pattern
> >> --
> >> 2.51.1
> >>