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