[PATCH] D60211: NFC: Refactor library-specific mappings of scalar maths functions to their vector counterparts

[Jinsong Ji via Phabricator via llvm-commits]

jsji added inline comments.


================
Comment at: llvm/include/llvm/Analysis/VecFuncs.def:13
+
+#if defined(TLI_DEFINE_VECDESCS)
+#define TLI_DEFINE_VECFUNC(SCAL, VEC, VF) {SCAL, VEC, VF},
----------------
pjeeva01 wrote:
> jsji wrote:
> > Why we want to check `TLI_DEFINE_VECDESCS` to define `TLI_DEFINE_VECFUNC`?
> > Can't we just check `#ifndef TLI_DEFINE_VECFUNC` here?
> The plan is to define TLI_DEFINE_VECFUNC in a different way when a different identifier (eg: TLI_DEFINE_ARR) is defined. 
> 
> https://reviews.llvm.org/D59881 shows the intention behind this structure. 
> 
> If you prefer for me to restructure this ifdef only to suit this patch, I will go ahead and change it.
> 
> 
OK, I think you meant `https://reviews.llvm.org/D59883`, where you want to extract the function name only.

Why not check `#ifdef TLI_DEFINE_VECFUNC` here, then we can add something like 

```
     #ifdef TLI_DEFINE_MASSV_VECFUNCS_NAMES  
     #define TLI_DEFINE_MASSV_VECFUNCS
     #define TLI_DEFINE_VECFUNC(SCAL, VEC, VF) VEC,
     #endif
```
before this to override the default, 
then use `#define TLI_DEFINE_MASSV_VECFUNCS_NAMES` in PPCLowerMASSVEntries.cpp ?

Then we will only need to define one macro before the `#include` in all places. 





================
Comment at: llvm/include/llvm/Analysis/VecFuncs.def:174
+#undef TLI_DEFINE_VECFUNC
+#undef TLI_DEFINE_ACCELERATE_VECFUNCS
+#undef TLI_DEFINE_SVML_VECFUNCS
----------------
pjeeva01 wrote:
> jsji wrote:
> > This is not always defined, should we call `#undef` when we are sure they are defined?
> The #undef is ignored if the specified identifier is not currently defined as a macro name. So, it should be alright to leave the #undef as it is.
> 
OK. Some old compiler (or pre-processor) might error due to this, but consider this is now in C standard now. It is OK to leave it as it is.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60211/new/

https://reviews.llvm.org/D60211