On Thu, Jul 06, 2023 at 02:48:19PM -0500, Peter Bergner wrote:
> On 7/6/23 12:33 PM, Segher Boessenkool wrote:
> > On Wed, Jul 05, 2023 at 05:21:18PM +0530, P Jeevitha wrote:
> >> --- a/gcc/config/rs6000/rs6000.cc
> >> +++ b/gcc/config/rs6000/rs6000.cc
> >> @@ -9894,6 +9894,8 @@ rs6000_legitimate_address_p (machine_mode mode, rtx
> >> x, bool reg_ok_strict)
> >>
> >> /* Handle unaligned altivec lvx/stvx type addresses. */
> >> if (VECTOR_MEM_ALTIVEC_OR_VSX_P (mode)
> >> + && mode != OOmode
> >> + && mode != XOmode
> >> && GET_CODE (x) == AND
> >> && CONST_INT_P (XEXP (x, 1))
> >> && INTVAL (XEXP (x, 1)) == -16)
> >
> > Why do we need this for OOmode and XOmode here, but not for the other
> > modes that are equally not allowed? That makes no sense.
>
> VECTOR_MEM_ALTIVEC_OR_VSX_P (mode) already filters those modes out
> (eg, SImode, DFmode, etc.), just not OOmode and XOmode, since those both
> are modes used in/with VSX registers.
It does not filter anything out, no. That simply checks if a datum of
that mode can be loaded into vector registers or not. For example
SImode could very well be loaded into vector registers! (It just is not
such a great idea).
And for some reason there is VECTOR_P8_VECTOR as well, which is mixing
multiple concepts already. Let's not add more, _please_.
> > Should you check for anything that is more than a register, for example?
> > If so, do *that*?
>
> Well rs6000_legitimate_address_p() is only passed the MEM rtx, so we have
> no idea if this is a load or store, so we're clueless on number of regs
> needed to hold this mode. The best we could do is something like
That is *bigger than* a register. It's the same in Dutch, sorry, I am
tired :-(
> GET_MODE_SIZE (mode) == GET_MODE_SIZE (V16QImode)
>
> or some such thing. Would you prefer something like that?
That is even worse :-(
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.target/powerpc/pr110411.c
> >> @@ -0,0 +1,21 @@
> >> +/* PR target/110411 */
> >> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -S -mblock-ops-vector-pair" }
> >> */
> >
> > -S in testcases is wrong. Why do you want this? It is *good* if this
> > is hauled through the assembler as well! If you *really* want this you
> > use "dg-do assemble", but you shouldn't.
>
> For test cases checking for ICEs, we don't need to assemble, so I agree,
> we just need to remove the -S option, which is implied by this being a
> dg-do compile test case (the default for this test directory).
We *do* want to assemble. It is a general principle that we want to
test as much as possible whenever possible. *Most* problems are found
with the help of testcases that were never designed for the problem
found!
dg-do compile *does* invoke the assembler, btw. As it should.
Segher
