On 2/4/21 3:16 PM, Segher Boessenkool wrote: > Hi! > > 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. > > This needs a documentation update. You can just delete the old names > there (i.e. rename everything there).
Good catch! I had meant to do that and totally forgot! :-( >> +#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. The first 11 times, we must make sure it expands to nothing/empty string and on the 12th time, it expands to create the bdesc_compat array. However, it looks like if you redefine it to be the same value each time, then you do not need to #undef it, so it looks like just the last usage when we create the bdesc_compat array will we need to #undef it. I'll give that a try and see how it works out. >> 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. Peter
