Message ID | 8f29097a-3690-5a60-ffb5-09382ed2f92e@codesourcery.com |
---|---|
State | New |
Headers | show |
Series | emit-rtl.c: Allow vector subreg of float vectors | expand |
Andrew Stubbs <ams@codesourcery.com> writes: > This patch is a prerequisite for some patches I have to add multiple > vector sizes on amdgcn. > > The problem is that validate_subreg rejects things like this: > > (subreg:V32SF (reg:V64SF) 0) > > These are commonly generated by the compiler. The integer equivalents > work fine. > > To be honest, I don't know why other targets have not encountered this > problem before? Perhaps because most other targets require all vectors > to have the same number of bits, meaning that float vectors have a fixed > number of lanes? In my case, amdgcn can convert V64SFmode to V32SFmode > by simply masking off some lanes. > > OK to commit? (I have an x86_64 bootstrap and test in progress.) > > Andrew > > Allow vector subreg of float vectors > > Integer vector subregs were already permitted. > > gcc/ChangeLog: > > * emit-rtl.c (validate_subreg): Permit sections of float vectors. > > diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c > index f9b0e9714d9..d7067989ad7 100644 > --- a/gcc/emit-rtl.c > +++ b/gcc/emit-rtl.c > @@ -947,6 +947,11 @@ validate_subreg (machine_mode omode, machine_mode imode, > else if (VECTOR_MODE_P (omode) > && GET_MODE_INNER (omode) == GET_MODE_INNER (imode)) > ; > + /* Allow sections of vectors, both smaller and paradoxical. (This just > + works for integers, but needs to be explicitly allowed for floats.) */ > + else if (VECTOR_MODE_P (omode) && VECTOR_MODE_P (imode) > + && GET_MODE_INNER (omode) == GET_MODE_INNER (imode)) > + ; Might be missing something, but isn't this a subcondition of the previous “else if”? It looks like that ought to catch every case that the new one does. Thanks, Richard > /* Subregs involving floating point modes are not allowed to > change size. Therefore (subreg:DI (reg:DF) 0) is fine, but > (subreg:SI (reg:DF) 0) isn't. */
On 06/08/2020 04:54, Richard Sandiford wrote: >> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c >> index f9b0e9714d9..d7067989ad7 100644 >> --- a/gcc/emit-rtl.c >> +++ b/gcc/emit-rtl.c >> @@ -947,6 +947,11 @@ validate_subreg (machine_mode omode, machine_mode imode, >> else if (VECTOR_MODE_P (omode) >> && GET_MODE_INNER (omode) == GET_MODE_INNER (imode)) >> ; >> + /* Allow sections of vectors, both smaller and paradoxical. (This just >> + works for integers, but needs to be explicitly allowed for floats.) */ >> + else if (VECTOR_MODE_P (omode) && VECTOR_MODE_P (imode) >> + && GET_MODE_INNER (omode) == GET_MODE_INNER (imode)) >> + ; > > Might be missing something, but isn't this a subcondition of the > previous “else if”? It looks like that ought to catch every case > that the new one does. Apparently, Hongtao and Uroš fixed my problem while I was working on this. Yes, my patch does the same (although I would question whether it's safe to use "GET_MODE_INNER (imode)" without having first confirmed "imode" is a vector mode). Anyway, I can drop my patch. Andrew
Andrew Stubbs <ams@codesourcery.com> writes: > On 06/08/2020 04:54, Richard Sandiford wrote: >>> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c >>> index f9b0e9714d9..d7067989ad7 100644 >>> --- a/gcc/emit-rtl.c >>> +++ b/gcc/emit-rtl.c >>> @@ -947,6 +947,11 @@ validate_subreg (machine_mode omode, machine_mode imode, >>> else if (VECTOR_MODE_P (omode) >>> && GET_MODE_INNER (omode) == GET_MODE_INNER (imode)) >>> ; >>> + /* Allow sections of vectors, both smaller and paradoxical. (This just >>> + works for integers, but needs to be explicitly allowed for floats.) */ >>> + else if (VECTOR_MODE_P (omode) && VECTOR_MODE_P (imode) >>> + && GET_MODE_INNER (omode) == GET_MODE_INNER (imode)) >>> + ; >> >> Might be missing something, but isn't this a subcondition of the >> previous “else if”? It looks like that ought to catch every case >> that the new one does. > > Apparently, Hongtao and Uroš fixed my problem while I was working on this. > > Yes, my patch does the same (although I would question whether it's safe > to use "GET_MODE_INNER (imode)" without having first confirmed "imode" > is a vector mode). FWIW, skipping the VECTOR_MODE_P test means that we also allow vector paradoxical subregs of scalars, which can be useful if you're trying to make a vector in which only element 0 is initialised. (Only works for element 0 though.) IIRC this is what some of the x86 patterns do. Guess it means we also allow vector subregs of complex values, but that kind-of seems OK/useful too. Thanks, Richard
Allow vector subreg of float vectors Integer vector subregs were already permitted. gcc/ChangeLog: * emit-rtl.c (validate_subreg): Permit sections of float vectors. diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c index f9b0e9714d9..d7067989ad7 100644 --- a/gcc/emit-rtl.c +++ b/gcc/emit-rtl.c @@ -947,6 +947,11 @@ validate_subreg (machine_mode omode, machine_mode imode, else if (VECTOR_MODE_P (omode) && GET_MODE_INNER (omode) == GET_MODE_INNER (imode)) ; + /* Allow sections of vectors, both smaller and paradoxical. (This just + works for integers, but needs to be explicitly allowed for floats.) */ + else if (VECTOR_MODE_P (omode) && VECTOR_MODE_P (imode) + && GET_MODE_INNER (omode) == GET_MODE_INNER (imode)) + ; /* Subregs involving floating point modes are not allowed to change size. Therefore (subreg:DI (reg:DF) 0) is fine, but (subreg:SI (reg:DF) 0) isn't. */