Hi!
On Mon, Jun 02, 2025 at 08:37:19PM +0200, Martin Uecker wrote:
> Am Montag, dem 02.06.2025 um 13:19 -0500 schrieb Segher Boessenkool:
> > On Mon, Jun 02, 2025 at 05:50:08PM +0200, Martin Uecker wrote:
> > > According to the discussion in the bugzilla there seems to be
>
noying, but the
requirements for the two are essentially the same, just the limit
"this is *too* annoying" shifted a bit.
Why do this in -Wc++-compat at all? You don't say. Well, you say that
is a bugfix, so it should be a separate (and trivial) patch.
Segher
On Thu, May 29, 2025 at 03:07:34PM -0500, Peter Bergner wrote:
> On 5/29/25 5:35 AM, Segher Boessenkool wrote:
> >
> > Add yourself to suthors as well?
>
> Agreed. Just add your name/email address directly under mine, like so:
>
> 2025-05-29 Peter Bergner
>
manually disabled things,
but that is not something we can accommodate)
> +/* Test whether the ISA 3.0 amo (atomic memory operations) functions perform
> as
> + expected. */
That sounds like this is a run test, but it isn't.
> diff --git a/gcc/testsuite/gcc.target/powerpc/amo4.c
> b/gcc/testsuite/gcc.target/powerpc/amo4.c
> new file mode 100644
> index 000..fce85c8dc52
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/amo4.c
> @@ -0,0 +1,92 @@
> +/* { dg-do run { target { lp64 && p9vector_hw } } } */
Please use a more appropriate selector?
> +/* { dg-options "-mdejagnu-cpu=power9 -O2" } */
> +/* { dg-require-effective-target powerpc_vsx } */
And get rid of this last one please.
Thanks,
Segher
ncludes as well :-)
Committing to trunk.
Segher
---
gcc/config/rs6000/rs6000.cc | 1 -
1 file changed, 1 deletion(-)
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 12dbde2bc630..9b01d3e29167 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
ill trap if one of the
> + arguments is a signalling NaN. */
> #define REVERSIBLE_CC_MODE(MODE) 1
Those insns *are* ordered comparison insns (in the Power ISA sense), and
they are unwanted for all the same reasons as we do not want the simpler
similar insns.
So please make a patch that instead of unnecessarily and wrongly
changing the fundamentals of how we handle FP comparisons in our
backend, just fixes the one pattern where we already messed up?
Segher
Hi!
On Wed, May 21, 2025 at 06:27:38AM +0200, Richard Biener wrote:
> On Wed, May 21, 2025 at 12:30 AM Segher Boessenkool
> wrote:
> >
> > On Mon, May 12, 2025 at 06:35:15PM -0400, Michael Meissner wrote:
> > > On Mon, May 12, 2025 at 01:24:04PM +0530, Surya Kumari Jan
is being thrown due to qNaNs.
>
> But -Ofast says not to worry about Nans (signalling or otherwise). But if
> Segher desires, I remove the test for Ofast.
That is not what -Ofast means at all. It means "-O3, but also
-ffast-math, and some other not recommendable things". Its nam
dentical, except for these two attributes. Which will automatically be
set to the correct values if you use simpler code, btw.
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr108958.c
> @@ -0,0 +1,27 @@
> +/* { dg-do compile } */
That's the default. You can leave this out.
So please make this work in way more cases, simpler code with better
results!
Segher
is being thrown due to qNaNs.
>
> But -Ofast says not to worry about Nans (signalling or otherwise). But if
> Segher desires, I remove the test for Ofast.
That is not what either of the five constituent options of -ffast-math
means. Any code should use (some of) those five options, almost
ct like that
> is still fine, you just get the pointer in a roundabout way. It would be
> different if the machine actually does a "keep following pointers until the
> 'this is a pointer' flag is false" mode, which some old machines do.
How should that even be expressed in GCC? Heh.
Segher
etter from an overall project maintenance
> standpoint, though it may not be the best solution for the targets which
> support double-indirect addressing modes.
It is pretty hard to work with double-indirect things, often have to
make sure the two memory accesses are not to the same address, etc.
Segher
up repeatedly - I can find the last
It was agreed that I could delete old reload completely in GCC 16.
I should have something ready in a week or so (almost everything needs
to be redone because of unrelated changes, again).
Segher
> proposal for GCC 13 and somehow remember we agreed to fina
would have been
really good if ports that have such difficulties had tried to use LRA
before! But here we are.
Segher
ever heve been a user option at all, it
either is okay or it isn't.
Maybe that thing should not be user-selectable at all.
("Maybe", like I guarantee I will okay a patch doing jsut that!)
(Not "would", but "will", thanks)
So yeah, tha patch is okay. Thank you!
Segher
On Tue, Mar 25, 2025 at 07:00:34PM -0500, Peter Bergner wrote:
> On 3/25/25 5:17 PM, Segher Boessenkool wrote:
> > On Tue, Mar 25, 2025 at 03:33:59PM -0500, Peter Bergner wrote:
> >> Segher, any reason you can give on why we shouldn't go the easy route to
> >>
ope :-)
> As shown here, they're pretty fragile to changes in the compiler.
Pretty much all scan-assembler-times tests are very suspect, not to say
plain wrong.
> That said, I'm not sure it's really worth splitting this older Power7
> test case up, so I guess adding -fno-ipa
; +/* { dg-options "-mdejagnu-cpu=power9 -O2" } */
> /* { dg-require-effective-target powerpc_vsx } */
This test is not typically needed at all, but yeah, it is possible some
tester decides to sabotage himself and disables VSX on new systems. The
extra check isn't really harmful anyway (it just might suggest to others
that it is required).
Okay for trunk and any backports you want. Thanks!
Segher
du)
>
> This doesn't make any sense to me. This is a new file with new
> code not copied form anywhere else, so why the Contributed by
> Richard Kenner line? Cut/paste error?
If a file is largely copied from some other file, it makes sense to keep
attribution. If not, not.
Same for that "1991-2024" btw. Dates should be factual.
Segher
On Tue, Nov 05, 2024 at 04:26:13PM -0600, Peter Bergner wrote:
> On 11/5/24 1:16 PM, Segher Boessenkool wrote:
> > Would it not have been better to not use -O0 at all here? -O0 is
> > not realistic if you expect any reasonable optimisation!
>
> That's what I first att
immune from any future
> changes in jump table generation behavior.
Please don't do such things.
Would it not have been better to not use -O0 at all here? -O0 is
not realistic if you expect any reasonable optimisation!
Segher
; and
Thank you! Okay for trunk; also okay for all backports (which backports
do you want?)
Segher
scan-assembler "stq \[123\]?\[02468\]" } } */
Please use {} quoting, and no backslashes. Also use \m and \M.
Or something like
scan-assembler { \mstq .*[02468], }
(you do not have to match the things you don't care about, and you only
need to look at the last digit to see if a number is even).
The patch looks good otherwise, but please fix these things and repost.
In a new thread :-)
Thanks,
Segher
On Wed, Sep 18, 2024 at 01:51:21AM -0400, Michael Meissner wrote:
> On Tue, Sep 17, 2024 at 02:33:09AM -0500, Segher Boessenkool wrote:
> > Hi!
> >
> > On Mon, Sep 16, 2024 at 11:40:45PM -0400, Michael Meissner wrote:
> > > With this patch, GCC now realizes that the
> +++ b/gcc/testsuite/gcc.target/powerpc/pr89213.c
> @@ -0,0 +1,106 @@
> +/* { dg-do compile { target { lp64 } } } */
Why only on 64-bit systems? Does it fail with -m32? Why / how?
With that, okay for trunk. Thanks!
Segher
or you don't care about that target, or just
cargo-cult, is wrong, and encourages more wrongness.
Segher
Also
okay with just the MODE_UNIT_SIZE (mode) == 16 thing, after you tested
that of course :-)
Thanks!
Segher
On Mon, Aug 12, 2024 at 10:59:01AM -0500, Peter Bergner wrote:
> Ping * 3. [Message-ID: <1e003d78-3b2e-4263-830a-7c00a3e9d...@linux.ibm.com>]
>
> Segher, this resolves the issues you mentioned in your review.
> This was on the top of your patch review queue before, so may
bit
even.
This is handy for people reading the ISA, like most of us: the
instruction descriptions talk about MSR[VEC] and MSR[VSX] all over the
place. And people will less easily understand this as being about just
the old insns!
Segher
Hi!
On Mon, Aug 12, 2024 at 10:42:48AM +0800, Kewen.Lin wrote:
> on 2024/8/10 05:43, Segher Boessenkool wrote:
> IIUC, we want to split TARGET_P[89]_VECTOR into TARGET_P[89]_ALTIVEC and
> TARGET_P[89]_VSX (or just TARGET_POWER[89] && TARGET_VSX or TARGET_ALTIVEC)
> according
well-formed, but it doesn't make sense, indeed :-) Named patterns
have requirements on their arguments, but everything else is whatever
the target wants :-)
Hopefully *no* passes will consider this a pure input, we have that "+"
after all! It would be better if it wasn't there, sure.
Segher
On Fri, Aug 09, 2024 at 03:50:50PM -0500, Peter Bergner wrote:
> On 8/9/24 12:54 PM, Segher Boessenkool wrote:
> >> --- a/gcc/config/rs6000/altivec.md
> >> +++ b/gcc/config/rs6000/altivec.md
> >> @@ -623,7 +623,7 @@ (define_insn "altivec_eqv1ti&q
ot a VSX insn (for which MSR[VSX]=1 is needed).
We test TARGET_ALTIVEC for that, not TARGET_VSX.
In general, we want to get rid of TARGET_Pxxx_VECTOR, not introduce new
stuff like it!
Segher
Hi!
On Wed, Jul 24, 2024 at 11:38:11AM -0700, Carl Love wrote:
> On 7/24/24 10:03 AM, Segher Boessenkool wrote:
> >So much manual stuff needed, sigh.
> >
> >On Fri, Jul 19, 2024 at 01:04:12PM -0700, Carl Love wrote:
> >>gcc/ChangeLog:
> >> * c
On Wed, Jul 24, 2024 at 12:16:33PM -0500, Peter Bergner wrote:
>
> On 7/24/24 12:03 PM, Segher Boessenkool wrote:
> >> +/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
> >
> > Why the -save-temps? Always document it if you want that for something,
On Wed, Jul 24, 2024 at 12:12:05PM -0500, Peter Bergner wrote:
> On 7/24/24 12:06 PM, Segher Boessenkool wrote:
> I thought we always wanted the predicate to match the constraint being used?
Predicates and constraints have different purposes, and are used at
different times (typ
R))]
> >"TARGET_POWER10"
>
> I know the old code used the register_operand predicate for the vector
> operands, but those really should be changed to altivec_register_operand.
register_operand is just fine usually. The "v" constraint already makes
sure things end up in a VMX (a lower VSX) register, the predicate
doesn't help here. register_operand is shorter (and thus, preferred),
and also more likely correct if the code changes later :-)
Segher
run, one for compiling only. It's
a bit simpler and cleaner.
> +/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
Why the -save-temps? Always document it if you want that for something,
but never put it in the testcase if not. A leftover from development?
Okay for trunk, thank you! Well Peter had some comments too, modulo
those I guess, I'll read them now ;-)
Segher
On Thu, Jul 18, 2024 at 09:53:05AM -0500, Peter Bergner wrote:
> On 7/18/24 8:23 AM, Segher Boessenkool wrote:
> > Everything in gcc.target/powerpc is only run for target powerpc*-*-*
> > anyway, so make this just
> >
> > /* { dg-do compile } */
> >
> >
-* } } */
> +/* Require VSX and Linux to eliminate systems where you can't do
> + __attribute__((__target_clones__(...))). */
All the same things here, mutatis mutandis.
Segher
R10 and POWER11 processors.
The official names are Power10 and Power11 (only the P a capital).
Okay for trunk with that fixed. Also okay for backport to 14. (After
the usual burn-in period of course). Thanks!
Segher
{powerpc}, @samp{powerpc64},
> -@samp{powerpc64le}, @samp{rs64}, and @samp{native}.
> +@samp{power9}, @samp{power10}, @samp{power11},
> +@samp{powerpc}, @samp{powerpc64}, @samp{powerpc64le},
> +@samp{rs64}, and @samp{native}.
Hint: you do not need to layout TeXinfo source code. TeX itself takes
care of that much better than you can. It is fine to keep some short
lines, this will be rewrapped in output anyway.
Okay for trunk with those changes. Also fine for backport to 14.
Thank you!
Segher
for -mcpu=power11 support.
>
> In order to use -mcpu=power11, you will need to use a new enough binutils that
> supports the .machine power11 option.
This is a general thing: we always assume you have a binutils at least
as new as your GCC.
Segher
al RTL name for it. That is fine, that is
what we do in many other places already. It is clear what is meant no
matter what :-)
Segher
see an existing optab for andn.
For most RTL stuff we can deal with it just fine using existing
define_insn etc. stuff. I have no idea if any of this is harder in
Gimple?
> So OK from my side in case there are no negative comments or
> bikeshedding on the name. I can't approve the rs6000 changes
> though.
But I can :-) I'll reply to just that. Thanks for handling this!
Segher
I didn't see this before. Sigh.
On Tue, Jan 02, 2024 at 09:47:11AM +, Richard Sandiford wrote:
> Segher Boessenkool writes:
> > On Tue, Oct 24, 2023 at 07:49:10PM +0100, Richard Sandiford wrote:
> >> This patch adds a combine pass that runs late in the pipeline.
>
works
best for you :-)
> Bootstrapped and regtested on powerpc64-linux-gnu P8/P9
> and powerpc64le-linux-gnu P9 and P10.
That also tested -m32 (on BE at least), right?
Okay for trunk, thanks for dealing with this!
Segher
On Tue, Jun 18, 2024 at 12:53:09PM -0500, Peter Bergner wrote:
> On 6/18/24 8:20 AM, Segher Boessenkool wrote:
> > On Mon, Jun 17, 2024 at 08:54:46PM -0500, Peter Bergner wrote:
> >> So we should be able to shrink-wrap in the presence of the ROP protection.
> [snip]
> &
the widen
things, don't try to do the compilers job of specialising stuff, it
only makes things much less readable, and causes more mistakes. Just do
like what was there before, essentially.
Segher
On Mon, Jun 17, 2024 at 08:54:46PM -0500, Peter Bergner wrote:
> On 6/17/24 7:57 PM, Segher Boessenkool wrote:
> > On Mon, Jun 17, 2024 at 06:49:18PM -0500, Peter Bergner wrote:
> >> On 6/17/24 6:11 PM, Segher Boessenkool wrote:
> >> Yeah, I didn't write that, I
Hi!
On Mon, Jun 17, 2024 at 06:49:18PM -0500, Peter Bergner wrote:
> On 6/17/24 6:11 PM, Segher Boessenkool wrote:
> >> - /* If we are inserting ROP-protect instructions, disable shrink wrap.
> >> */
> >> - if (rs6000_rop_protect)
> >> -flag_shrink_
p. */
> - if (rs6000_rop_protect)
> -flag_shrink_wrap = 0;
> }
(Yes, I know the original code didn't say either, but let's try to make
things better :-) )
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr114759-1.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mrop-protect
> -fdump-rtl-pro_and_epilogue" } */
> +/* { dg-require-effective-target rop_ok } */
Do you want rop_ok while you are *forcing* it to be okay anyway? Why?
Segher
e compiler with LTO to undo that optimization. */
Some of these array names no longer have the rs6000_ prefix now. Oh
wait, you already took that into account? I'm not saying anything :-)
The patch is fine for trunk, thank you! If you want backports those
are okay, too (but I don't think you want any? Or does this work
withput the previous patches as well?)
Segher
p the size of the
> rs6000_init_generated_builtins function quite a lot.
> The above patch decreases it back, to even less than the size of
> the function before my fix.
A patch in the middle of a thread. I missed it, sorry. Please send
patches as separate threads?
Segher
/
> +/* { dg-options "-O2 -mdejagnu-cpu=power8" } */
> +/* { dg-require-effective-target p8vector_hw } */
I have no idea why the original didn't do -O2 already, heh. So this is
only an improvement, right! I won't complain at all unless it fails :-)
Segher
! has_arch_pwr8
> } } } */
> /* { dg-require-effective-target powerpc_vsx } */
Everything in gcc.target/powerpc/ is tested for "target powerpc*-*-*"
already, so you could remove that target clause even (after testing of
course :-) )
Okay for trunk with or without that extra tweak. Thank you!
Segher
On Wed, Jun 12, 2024 at 07:02:31PM -0500, Peter Bergner wrote:
> On 6/12/24 3:00 PM, Segher Boessenkool wrote:
> >> /* { dg-do compile { target { powerpc64*-*-* } } } */
> >
> > Probably should be an "lp64" instead?
>
> Actually, there is nothing in
} 1 } } */
> +/* { dg-final { scan-assembler-times {\m(?:xxsel|xxlor|vor)\M} 1 } } */
> /* { dg-final { scan-assembler-not {\mvspltisw\M} } } */
> /* { dg-final { scan-assembler-not {\mxxlorc\M} } } */
Okido, thanks! The three trivial tweaks I suggest here are okay as
well if you want, after testing of course ;-)
Segher
0;
> +}
> +
> +/* { dg-final { scan-assembler "power4" } } */
Please explain (in the testcase, not here!) what this is meant to test!
You probably want to say {\mpower4\M} instead, btw. Unless you want to
match ".machine spower436" as well?
Segher
he constant into a register", yes.
> --- a/gcc/testsuite/gcc.target/powerpc/pr106550_1.c
> +++ b/gcc/testsuite/gcc.target/powerpc/pr106550_1.c
> @@ -4,17 +4,19 @@
> /* { dg-options "-O2 -mdejagnu-cpu=power10 -fdisable-rtl-split1" } */
> /* force the constant splitter run after RA: -fdisable-rtl-split1. */
>
> +/* The below marco helps to avoid using paddi and avoid loading from memory.
> */
(macro)
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr63281.c
> @@ -0,0 +1,11 @@
> +/* Check loading constant from memory pool. */
> +/* { dg-options "-O2 -mpowerpc64" } */
> +
> +void
> +foo (unsigned long long *a)
> +{
> + *a++ = 0x2351847027482577ULL;
> +}
> +
> +/* { dg-final { scan-assembler-times {\mp?ld\M} 1 { target { lp64 } } } } */
Why target lp64 only? You have -mpowerpc64 already, does that not make
us use ld here always?
Segher
ose to actually
include this patch :-)
> Bootstrapped/regtested on powerpc64le-linux and powerpc64-linux (-m32/-m64
> testing there), ok for trunk and after a while for release branches?
Yes please. What nastiness. Thanks for dealing with it!
Segher
> 2024-06-03 Jakub Jelinek
On Fri, May 31, 2024 at 09:14:21AM +0100, Richard Sandiford wrote:
> Segher Boessenkool writes:
> > Hi!
> >
> > On Fri, May 31, 2024 at 01:21:44AM +0530, Ajit Agarwal wrote:
> >> Code is implemented with pure virtual functions to interface with target
> >>
s would be WAY easier to review (read: AT ALL POSSIBLE) if you
included some detailed rationale and design document.
Segher
Hi Alex,
On Thu, May 30, 2024 at 01:40:27PM -0300, Alexandre Oliva wrote:
> Sorry, I misnumbered this patch as #1/2 when first posting v3.
I see at least five completely different patches in this email thread,
with different subjects and all.
> On May 28, 2024, Segher Boessenkool
nu. Are these the sort of test case you're interested
> in, or are you looking for something that tests the offsets in debug
> info, rather than the end-to-end debugging feature?
A testcase specifically for this would be good, yes. Where you can say
at the top of the file "This tests that ... [X and Y]" :-)
Segher
he builtin, right? So write that instead?
"Return 1 if the operand (a scalar floating poiint number) is finite",
or such?
Segher
s what it defines;
> > 3) optab defined, FAIL, generate a library call;
> >
> > From above, I had the concern that ports may assume FAILing can
> > fall back with the generic folding, but it's not actually.
>
> Hmm, but it should. Can you make that work?
That certainly would be the least surprising!
Segher
maybe, but don't try to "optimise" that expression, let the compiler
do that, it is much better at it anyway :-) )
> +}
Is that correct for all ABIs we support? Even if so, it needs a lot
more documentation than this.
Segher
On Sat, May 25, 2024 at 09:12:05AM -0300, Alexandre Oliva wrote:
You sent multiple patch series in one thread, and multiple versions of
the same series even.
This is very hard to even *read*, let alone work with. Please don't.
Segher
rget-supports.exp change plus
> there is no mention of why it's needed in the git log entry.
In the commit message you mean? Yeah. This info belongs in the commit
message.
Is the target-supports thing that Cell thing? That does not belong here
at all. If it wasn't simply a mistake, it should be a separate commit,
with a lot of explanation.
Segher
that, heh.
But it's fine, the insn patterns it uses use the same conditions
already.
Segher
only? If there is a reason, document
that here (but is there a reason?)
> +/* { dg-require-effective-target ppc_float128_sw } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power9 -mvsx -mabi=ieeelongdouble
> -Wno-psabi" } */
Same comments here: If you have a -mcpu you do not want vsx_ok or -mvsx.
Please fix these things and resend. Thanks!
Segher
Hi!
On Thu, May 16, 2024 at 02:56:49PM +0800, Jiufu Guo wrote:
> Jiufu Guo writes:
> > Segher Boessenkool writes:
> >> On Tue, May 14, 2024 at 05:53:56PM +0800, Jiufu Guo wrote:
> >>> Thanks so much for your great review!
> >>> Reference other messages
ory operand"? Maybe even print out the actual
operand given, too.
Segher
error messages you do not often know what caused the problem, so
just report on the facts you *do* know (and moreso with warnings, there
you typically only know something looks unusual).
Segher
roblem after all, and giving the
user the same error message is the most helpful thing to do!
It can be useful to not say "ICE", but it already is prevented from
doing that here.
Segher
u
*have to* have mem here?
The code you add that tests for address_operand looks wrong. I would
expect it to test the operand is memory, instead :-)
Segher
.
Most things already use only insn_cost, if you have the appropriate
hooks implemented in your backend (all one of them even). This is so
much easier to use (you only need to recognise some big categories of
instructions, for a typical target core, instead of eighty different
RTX codes, and the combination of them), and gives way better results.
Segher
On Fri, May 10, 2024 at 12:19:35PM +0200, Richard Biener wrote:
> On Fri, May 10, 2024 at 11:06 AM Segher Boessenkool
> wrote:
> > *All* code using a cost will have to be inspected and possibly adjusted
> > if you decide to use a different value for "unknown" than w
eptional value would have been -1, avoiding the compare).
> But sometime it adds an insn cost. If the unknown cost is -1, the total cost
> might be distorted.
*All* code using a cost will have to be inspected and possibly adjusted
if you decide to use a different value for "unknown" than what we have
had for ages. All other cost functions interacting with this one, too.
Segher
st return zero (unknown), so the
> comment holds to pattern_cost the same (it returns an 'int' so the better
> exceptional value would have been -1, avoiding the compare).
Cost 0 for unkown is used in (almost) *all* cost things in RTL. Pretty
much everything can deal with it just fine. What is different here?
The abstraction "pattern_cost" is a lousy abstraction of course, but
where is this used? Cost "unknown" can be passed through everywhere,
in principle.
Segher
same true for many other objects that are initialized
> lazily?
For all, even.
Without this change the compiler can (in theory, anyway) diagnose the
uninitialised use. After, there *is* no uninitialised use anymore.
Please don't do this. It is not an improvement, it is several steps
back, to satisfy a misguides sense of "security".
Segher
It does not
make any sense.
Is there any place where newpat is used uninitialised?
Segher
erence at all anyway,
it always has set the invalid flag after all.
Thanks!
Segher
> to be used but
> > it's a bit more meaningful (especially we already have mpower10),
> > theoretically speaking
> > it's undocumented users shouldn't use it at all.
>
> Sorry, I should have mentioned this, but I originally had it -mpower8, but
> given
> it wa
On Wed, Apr 10, 2024 at 08:32:39PM +0200, Uros Bizjak wrote:
> On Wed, Apr 10, 2024 at 7:56 PM Segher Boessenkool
> wrote:
> > This is never okay. You cannot commit a patch without approval, *ever*.
This is the biggest issue, to start with. It is fundamental.
> > That pat
on.
This is never okay. You cannot commit a patch without approval, *ever*.
That patch is also obvious -- obviously *wrong*, that is. There are
big assumptions everywhere in the compiler how a CC reg can be used.
This violates that, as explained elsewhere.
Segher
had to share it! Worth the
> testsuite churn anyway. :)
>
> Segher, if you end up reverting r14-9692-g839bc42772ba7a (as
> unfortunately seems not unlikely), then please also revert this
> commit: r14-9799-g4c8b3600c4856f7915281ae3ff4d97271c83a540.
I won't revert it, it fixes
but we need to do an actual uncse first!
Somewhere before combine, and then redo a CSE after it. An actual CSE,
not doing ten gazillion other things.
Segher
this is not the
way to do it.
Committed to trunk.
Segher
2024-03-27 Segher Boessenkool
PR rtl-optimization/101523
* combine.cc (try_combine): Don't do a 2-insn combination if
it does not in fact change I2.
---
gcc/combine.cc | 11 +++
1 file change
; register here.
So you can use an unspec just to convert the flags reg to an integer?
To make an integer representation of flags reg contents.
Or is that what we started with here?
Segher
treme example look at 390, but on pretty
much any target both signed and unsigned comparisons use the same flag
bits, and maybe fp comparisons as well.
But pushfl does sound like pure dataflow. Why is this a builtin, is
it ever a good idea if the user writes stuff the compiler can do a
better job doing itself, not to mention it is way easier for the
compiler anyway? :-)
Segher
RS6000_BTI_ptr_long_long_unsigned,
> + RS6000_BTI_PTI,
Please call it RS6000_BTI_INTPTI, to be more in line with the naming of
other things.
With that fixed it should be good. Please repost with a good commit
comment and changelog :-)
Thanks,
Segher
On Thu, Mar 07, 2024 at 11:07:18PM +0100, Uros Bizjak wrote:
> On Thu, Mar 7, 2024 at 10:37 PM Segher Boessenkool
> wrote:
> > > but can be something else, such as the above noted
> > >
> > > (unspec:DI [
> > > (reg:CC 17 flags)
> > >
w you want me
> to structure the patches.
*You* should know the code already, so you surely can figure out a nice
way to present it, so that it takes me LESS work to review this than it
took you to write it?
Making a patch series reviewable is part of the development effort. It
is way less work if you start with this as the goal in mind. It is less
work than writing (and debugging etc.) an omnibus patch, in the first
place!
Your goal is to make a patch series that will be reviewed and then seen
to be great stuff. So it of course needs to *be* great stuff, but it
also needs to be presented in such a way that that is obvious.
Segher
loc, 0), ...)
>
> on unspec, which has no XEXP (..., 0).
>
> And *this* is what triggers RTX checking assert.
The unspec should have the CC compared with 0 as argument.
Segher
ses.
Yup. That is the whole point of find_single_use: if that test fails,
combine knows to get its paws off :-)
Segher
taken care of that ;-)
All as documented.
Segher
On Thu, Mar 07, 2024 at 11:22:12AM +0100, Richard Biener wrote:
> > > > Undo the combination if *cc_use_loc is not COMPARISON_P.
Why, anyway? COMPARISON_P means things like LE. It does not even
include actual RTX COMPARE.
Segher
&cc_use_insn))
> + && COMPARISON_P (*cc_use_loc))
Line 3167 already is
&& GET_CODE (SET_SRC (PATTERN (i3))) == COMPARE
so what in your backend is unusual?
Segher
1 - 100 of 1175 matches
Mail list logo
