On 8/26/24 10:39 AM, Segher Boessenkool wrote: > On Thu, Aug 22, 2024 at 05:39:36PM +0800, Kewen.Lin wrote: >>> - if (ALTIVEC_OR_VSX_VECTOR_MODE (mode) || mode == TImode) >>> + if (ALTIVEC_OR_VSX_VECTOR_MODE (mode) || mode == TImode >>> + || mode == PTImode) >> >> Maybe we can introduce a macro to this file like >> >> #define TI_OR_PTI_MODE(mode) ((mode) == TImode || (mode) == PTImode) > > INTEGRAL_MODE_P (mode) && MODE_UNIT_SIZE (mode) == 16 or such?
So I had already pushed the code using "((mode) == TImode || (mode) == PTImode)" by the time you replied. I think your suggestion would work too and I could change it to that if you want. That would make it future proof when we have PPTImode, PTTImode, ...! :-) That said, maybe the TI_OR_PTI_MODE name isn't the best name if we will have another 128-bit integer mode in the future. Maybe something like: #define INT128_MODE_P(mode) (INTEGRAL_MODE_P (mode) && MODE_UNIT_SIZE (mode) == 16) ...or some such name? Thoughts? I'll give it a spin through bootstrap and regtesting anyway. > Or you might just want the check for 16, that covers all applicable > modes and nothing else, right? IIRC, I think we only want integer modes here, so I think we need more than just the mode size check so we don't match against TD/TF/KF/IF modes. > I don't like macros that you use just one or two times. It is much > clearer if you write it out whereever you use it. I think Kewen's thought was (and I agree) is that we have a fair amount of code that handles TImode that should probably handle both TImode and PTImode together, so even though we currently only have one use of the macro, the number of uses should increase as we clean up the other areas than should handle PTImode too. Currently, I think the only time PTImode shows up is through use of the __atomic* intrinsics, but when Jeevitha's patch that allows the kernel to use an attribute to create PTImode variables so they can have higher alignment requirements, the number of PTImode uses we see will probably increase. Peter
