[PATCH] rs6000: Fix MMA API - Add support for compatibility built-ins

Segher Boessenkool segher@kernel.crashing.org
Fri Feb 5 18:25:27 GMT 2021


On Thu, Feb 04, 2021 at 10:05:19PM -0600, Peter Bergner wrote:
> On 2/4/21 3:16 PM, Segher Boessenkool wrote:
> > On Thu, Feb 04, 2021 at 02:40:20PM -0600, Peter Bergner wrote:
> >> The LLVM and GCC teams agreed to rename the __builtin_mma_assemble_pair and
> >> __builtin_mma_disassemble_pair built-ins to __builtin_vsx_assemble_pair and
> >> __builtin_vsx_disassemble_pair respectively.  It's too late to remove the
> >> old names, so this patch adds support for creating compatibility built-ins
> >> (ie, multiple built-in functions generate the same code) and then creates
> >> compatibility built-ins using the new names.

> >> +#ifndef RS6000_BUILTIN_COMPAT
> >> +  #undef BU_COMPAT
> >> +  #define BU_COMPAT(ENUM, COMPAT_NAME)
> > 
> > Please do not do #undef unless necessary: it hides bugs (and that causes
> > more bugs).
> 
> I thought it was needed, since rs6000-builtin.def is #included into
> rs6000-call.c and rs6000.h multiple times.

Ah yes, that is a good enough reason.  Bill's rewrite will clean all
this (and much more) up anyway, so it is fine for now.

> >>  BU_MMA_3 (ASSEMBLE_PAIR,    "assemble_pair",	MISC, mma_assemble_pair)
> >> +BU_COMPAT (MMA_BUILTIN_ASSEMBLE_PAIR, "vsx_assemble_pair")
> > 
> > You should do those the other way around (the mma_ one is the
> > compatibility one).  This matters, because if you disable the
> > compatibility builtins the vsx_ one should still be there, but not the
> > old name.  (It also makes more sense of course).
> 
> I actually did it that way initially, but decided to do it this was
> just to make the patch smaller.  You are correct that it's "more"
> correct to rename it though. 

It is less work (less confusion) in the future, and the number of lines
isn't important anyway :-)


Segher


More information about the Gcc-patches mailing list