This is an archive of the discontinued LLVM Phabricator instance.

NFC: Refactor library-specific mappings of scalar maths functions to their vector counterparts
ClosedPublic

Authored by pjeeva01 on Apr 3 2019, 9:24 AM.

Details

Summary

This patch factors out mappings of scalar maths functions to their vector counterparts from TargetLibraryInfo.cpp to a separate VecFuncs.def file. Such mappings are currently available for Accelerate framework, and SVML library.

This patch is a precursor to Initial support for vectorization using MASS (IBM MASS vector library).

Diff Detail

Repository
rL LLVM

Event Timeline

pjeeva01 created this revision.Apr 3 2019, 9:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 3 2019, 9:24 AM
jsji added inline comments.Apr 3 2019, 12:18 PM
llvm/include/llvm/Analysis/VecFuncs.def
13 ↗(On Diff #193524)

Why we want to check TLI_DEFINE_VECDESCS to define TLI_DEFINE_VECFUNC?
Can't we just check #ifndef TLI_DEFINE_VECFUNC here?

174 ↗(On Diff #193524)

This is not always defined, should we call #undef when we are sure they are defined?

pjeeva01 marked 2 inline comments as done.Apr 3 2019, 12:36 PM
pjeeva01 added inline comments.
llvm/include/llvm/Analysis/VecFuncs.def
13 ↗(On Diff #193524)

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.

174 ↗(On Diff #193524)

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.

jsji added inline comments.Apr 3 2019, 1:15 PM
llvm/include/llvm/Analysis/VecFuncs.def
13 ↗(On Diff #193524)

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.

174 ↗(On Diff #193524)

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.

pjeeva01 updated this revision to Diff 193724.Apr 4 2019, 8:24 AM

This updated patch uses a single macro to define the mapping from scalar maths functions to their vector counterparts ( as suggested in the earlier review of the original patch).

jsji accepted this revision.Apr 4 2019, 8:47 AM

LGTM. Thanks for refactoring.

This revision is now accepted and ready to land.Apr 4 2019, 8:47 AM
nemanjai accepted this revision.Apr 5 2019, 4:41 AM

LGTM. Thanks for pulling out this change into a separate patch. I'll commit it for you on Monday. This way others have an extra opportunity to add any comments or concerns.

This revision was automatically updated to reflect the committed changes.