Hi! On Tue, Sep 03, 2019 at 05:01:46PM -0400, Michael Meissner wrote: > On Thu, Aug 29, 2019 at 12:02:05PM -0500, Segher Boessenkool wrote: > > On Tue, Aug 20, 2019 at 02:00:31PM -0400, Michael Meissner wrote: > > > The test for TARGET_VADDUQM > > > matches a test earlier in the function where VSX registers are checked. > > > > Yeah, but does that test make any sense? > > The test is copying the same test used earlier for VSX support. As I said > without the patch, the reg_addr tables thinks that V1TImode can got in Altivec > registers, even if Altivec registers aren't allowed. It is a nit, and other > things prevent the type from being allocated. > > > Why p8 (or later) only? Why vector only? (Well that one is clear, and > > what this patch is about). Why -mpowerpc64 only? Because it has __int128 > > maybe? But then you should test for *that* (and why is it important?) > > Well -mpower64 is needed because the only way you can get __int128 types is by > being 64-bit (VITImode is of course vector __int128).
But types are not modes. > The history is this mode is a hack. It was added in the power8 time frame, > solely for the following built-in functions: > > __builtin_altivec_vadduqm > __builtin_altivec_vaddcuq > __builtin_altivec_vsubuqm > __builtin_altivec_vsubcuq > __builtin_altivec_vaddeuqm > __builtin_altivec_vaddecuq > __builtin_altivec_vsubeuqm > __builtin_altivec_vsubecuq > > At the time, TImode (aka, __int128) was not allowed in VSX registers since we > were using the original register allocator (reload based). There were several > issues that showed up in building large programs if we allowed TImode into > vector registers. So we disallowed it (with a chicken switch to re-enable it > for debugging, -mvsx-timode). > > Later when we switched over to the LRA register allocator, we allowed TImode > to > go into vector registers. But the GCC release had gone out that said if you > wanted to use those two power8 instructions you had to use a vector __int128 > wrapper to allow the code. > > > Where would V1TI go if not in vector regs? Just in memory? > > Since the only real use of it is for the built-ins, it likely won't matter. > But like all other modes, it could easily go in vector registers. > > Personally except for the register allocation issue, I don't see any reason > you > should use a vector type that is as large as the vector element inside. > > > > > And, what happens on 970? p5 doesn't have vector registers, but 970 does. > > You can't declare the type using the normal sequence, since rs6000-c.c does > not > allow 'vector __int128' until power8. If instead, you write out: Ah, so that is why you test for p8 as well. So you need neither the p8 nor the -mpowerpc64 test here... Can you just test TARGET_VSX instead? Or whatever else is actually needed? > vector __int128 > as > __attribute__((altivec(vector__))) __int128 > > it will use GPRs to hold it on -mcpu=970 (with/without -maltivec). On power7 > it will use the vector register. On power6 with -maltivec, it will use GPRs. > > > > --- gcc/config/rs6000/rs6000.c (revision 274635) > > > +++ gcc/config/rs6000/rs6000.c (working copy) > > > @@ -1874,7 +1874,7 @@ rs6000_hard_regno_mode_ok_uncached (int > > > /* AltiVec only in AldyVec registers. */ > > > if (ALTIVEC_REGNO_P (regno)) > > > return (VECTOR_MEM_ALTIVEC_OR_VSX_P (mode) > > > - || mode == V1TImode); > > > + || (TARGET_VADDUQM && mode == V1TImode)); Or is nothing needed? Now I don't understand anymore :-/ ... rs6000_vector_mem[V1TImode] = VECTOR_VSX; if TARGET_VSX so that is all you need? Then VECTOR_MEM_ALTIVEC_OR_VSX_P (V1TImode) is true already, so we do not need to handle V1TImode specially here at all? That is, if your patch is correct. So the question then is do we ever use V1TI if we do not have VSX. Segher
