[llvm] r240794 - Fix ODR violation waiting to happen by making static function definitions in VectorUtils.h non-static and defined out of line

Nema, Ashutosh Ashutosh.Nema at amd.com
Mon Jun 29 21:21:22 PDT 2015


Sorry Ahmed about this, I did not expected this build failure.
Thanks David for moving these files in Analysis.

Regards,
Ashutosh

-----Original Message-----
From: Ahmed Bougacha [mailto:ahmed.bougacha at gmail.com] 
Sent: Friday, June 26, 2015 10:52 PM
To: David Blaikie; Nema, Ashutosh
Cc: LLVM Commits
Subject: Re: [llvm] r240794 - Fix ODR violation waiting to happen by making static function definitions in VectorUtils.h non-static and defined out of line

Hi David, Ashutosh,

Should VectorUtils be in Analysis instead?  Seems like it doesn't
transform anything, and I'm having (shared) link failures on OS X:

Undefined symbols for architecture x86_64:
  "llvm::getIntrinsicIDForCall(llvm::CallInst*,
llvm::TargetLibraryInfo const*)", referenced from:
      llvm::LoopAccessInfo::analyzeLoop(llvm::DenseMap<llvm::Value
const*, llvm::Value*, llvm::DenseMapInfo<llvm::Value const*>,
llvm::detail::DenseMapPair<llvm::Value const*, llvm::Value*> > const&)
in LoopAccessAnalysis.cpp.o
ld: symbol(s) not found for architecture x86_64

Per the various LLVMBuild.txts, TransformUtils already depends on
Analysis, so having Analysis depend on TransformUtils won't fly
(llvm-build complains about cycles).

-Ahmed


On Fri, Jun 26, 2015 at 9:57 AM, David Blaikie <dblaikie at gmail.com> wrote:
> Author: dblaikie
> Date: Fri Jun 26 11:57:30 2015
> New Revision: 240794
>
> URL: http://llvm.org/viewvc/llvm-project?rev=240794&view=rev
> Log:
> Fix ODR violation waiting to happen by making static function definitions in VectorUtils.h non-static and defined out of line
>
> Patch by Ashutosh Nema
>
> Differential Revision: http://reviews.llvm.org/D10682
>
> Added:
>     llvm/trunk/lib/Transforms/Utils/VectorUtils.cpp
>       - copied, changed from r240790, llvm/trunk/include/llvm/Transforms/Utils/VectorUtils.h
> Modified:
>     llvm/trunk/include/llvm/Transforms/Utils/VectorUtils.h
>     llvm/trunk/lib/Transforms/Utils/CMakeLists.txt
>
> Modified: llvm/trunk/include/llvm/Transforms/Utils/VectorUtils.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/VectorUtils.h?rev=240794&r1=240793&r2=240794&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Transforms/Utils/VectorUtils.h (original)
> +++ llvm/trunk/include/llvm/Transforms/Utils/VectorUtils.h Fri Jun 26 11:57:30 2015
> @@ -21,184 +21,35 @@
>  namespace llvm {
>
>  /// \brief Identify if the intrinsic is trivially vectorizable.
> -///
>  /// This method returns true if the intrinsic's argument types are all
>  /// scalars for the scalar form of the intrinsic and all vectors for
>  /// the vector form of the intrinsic.
> -static inline bool isTriviallyVectorizable(Intrinsic::ID ID) {
> -  switch (ID) {
> -  case Intrinsic::sqrt:
> -  case Intrinsic::sin:
> -  case Intrinsic::cos:
> -  case Intrinsic::exp:
> -  case Intrinsic::exp2:
> -  case Intrinsic::log:
> -  case Intrinsic::log10:
> -  case Intrinsic::log2:
> -  case Intrinsic::fabs:
> -  case Intrinsic::minnum:
> -  case Intrinsic::maxnum:
> -  case Intrinsic::copysign:
> -  case Intrinsic::floor:
> -  case Intrinsic::ceil:
> -  case Intrinsic::trunc:
> -  case Intrinsic::rint:
> -  case Intrinsic::nearbyint:
> -  case Intrinsic::round:
> -  case Intrinsic::bswap:
> -  case Intrinsic::ctpop:
> -  case Intrinsic::pow:
> -  case Intrinsic::fma:
> -  case Intrinsic::fmuladd:
> -  case Intrinsic::ctlz:
> -  case Intrinsic::cttz:
> -  case Intrinsic::powi:
> -    return true;
> -  default:
> -    return false;
> -  }
> -}
> -
> -static inline bool hasVectorInstrinsicScalarOpd(Intrinsic::ID ID,
> -                                         unsigned ScalarOpdIdx) {
> -  switch (ID) {
> -    case Intrinsic::ctlz:
> -    case Intrinsic::cttz:
> -    case Intrinsic::powi:
> -      return (ScalarOpdIdx == 1);
> -    default:
> -      return false;
> -  }
> -}
> -
> -static Intrinsic::ID checkUnaryFloatSignature(const CallInst &I,
> -                                              Intrinsic::ID ValidIntrinsicID) {
> -  if (I.getNumArgOperands() != 1 ||
> -      !I.getArgOperand(0)->getType()->isFloatingPointTy() ||
> -      I.getType() != I.getArgOperand(0)->getType() ||
> -      !I.onlyReadsMemory())
> -    return Intrinsic::not_intrinsic;
> -
> -  return ValidIntrinsicID;
> -}
> -
> -static Intrinsic::ID checkBinaryFloatSignature(const CallInst &I,
> -                                               Intrinsic::ID ValidIntrinsicID) {
> -  if (I.getNumArgOperands() != 2 ||
> -      !I.getArgOperand(0)->getType()->isFloatingPointTy() ||
> -      !I.getArgOperand(1)->getType()->isFloatingPointTy() ||
> -      I.getType() != I.getArgOperand(0)->getType() ||
> -      I.getType() != I.getArgOperand(1)->getType() ||
> -      !I.onlyReadsMemory())
> -    return Intrinsic::not_intrinsic;
> -
> -  return ValidIntrinsicID;
> -}
> -
> -static Intrinsic::ID
> -getIntrinsicIDForCall(CallInst *CI, const TargetLibraryInfo *TLI) {
> -  // If we have an intrinsic call, check if it is trivially vectorizable.
> -  if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(CI)) {
> -    Intrinsic::ID ID = II->getIntrinsicID();
> -    if (isTriviallyVectorizable(ID) || ID == Intrinsic::lifetime_start ||
> -        ID == Intrinsic::lifetime_end || ID == Intrinsic::assume)
> -      return ID;
> -    else
> -      return Intrinsic::not_intrinsic;
> -  }
> -
> -  if (!TLI)
> -    return Intrinsic::not_intrinsic;
> -
> -  LibFunc::Func Func;
> -  Function *F = CI->getCalledFunction();
> -  // We're going to make assumptions on the semantics of the functions, check
> -  // that the target knows that it's available in this environment and it does
> -  // not have local linkage.
> -  if (!F || F->hasLocalLinkage() || !TLI->getLibFunc(F->getName(), Func))
> -    return Intrinsic::not_intrinsic;
> -
> -  // Otherwise check if we have a call to a function that can be turned into a
> -  // vector intrinsic.
> -  switch (Func) {
> -  default:
> -    break;
> -  case LibFunc::sin:
> -  case LibFunc::sinf:
> -  case LibFunc::sinl:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::sin);
> -  case LibFunc::cos:
> -  case LibFunc::cosf:
> -  case LibFunc::cosl:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::cos);
> -  case LibFunc::exp:
> -  case LibFunc::expf:
> -  case LibFunc::expl:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::exp);
> -  case LibFunc::exp2:
> -  case LibFunc::exp2f:
> -  case LibFunc::exp2l:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::exp2);
> -  case LibFunc::log:
> -  case LibFunc::logf:
> -  case LibFunc::logl:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::log);
> -  case LibFunc::log10:
> -  case LibFunc::log10f:
> -  case LibFunc::log10l:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::log10);
> -  case LibFunc::log2:
> -  case LibFunc::log2f:
> -  case LibFunc::log2l:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::log2);
> -  case LibFunc::fabs:
> -  case LibFunc::fabsf:
> -  case LibFunc::fabsl:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::fabs);
> -  case LibFunc::fmin:
> -  case LibFunc::fminf:
> -  case LibFunc::fminl:
> -    return checkBinaryFloatSignature(*CI, Intrinsic::minnum);
> -  case LibFunc::fmax:
> -  case LibFunc::fmaxf:
> -  case LibFunc::fmaxl:
> -    return checkBinaryFloatSignature(*CI, Intrinsic::maxnum);
> -  case LibFunc::copysign:
> -  case LibFunc::copysignf:
> -  case LibFunc::copysignl:
> -    return checkBinaryFloatSignature(*CI, Intrinsic::copysign);
> -  case LibFunc::floor:
> -  case LibFunc::floorf:
> -  case LibFunc::floorl:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::floor);
> -  case LibFunc::ceil:
> -  case LibFunc::ceilf:
> -  case LibFunc::ceill:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::ceil);
> -  case LibFunc::trunc:
> -  case LibFunc::truncf:
> -  case LibFunc::truncl:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::trunc);
> -  case LibFunc::rint:
> -  case LibFunc::rintf:
> -  case LibFunc::rintl:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::rint);
> -  case LibFunc::nearbyint:
> -  case LibFunc::nearbyintf:
> -  case LibFunc::nearbyintl:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::nearbyint);
> -  case LibFunc::round:
> -  case LibFunc::roundf:
> -  case LibFunc::roundl:
> -    return checkUnaryFloatSignature(*CI, Intrinsic::round);
> -  case LibFunc::pow:
> -  case LibFunc::powf:
> -  case LibFunc::powl:
> -    return checkBinaryFloatSignature(*CI, Intrinsic::pow);
> -  }
> +bool isTriviallyVectorizable(Intrinsic::ID ID);
>
> -  return Intrinsic::not_intrinsic;
> -}
> +/// \brief Identifies if the intrinsic has a scalar operand. It checks for
> +/// ctlz,cttz and powi special intrinsics whose argument is scalar.
> +bool hasVectorInstrinsicScalarOpd(Intrinsic::ID ID, unsigned ScalarOpdIdx);
> +
> +/// \brief Identify if call has a unary float signature
> +/// It returns input intrinsic ID if call has a single argument,
> +/// argument type and call instruction type should be floating
> +/// point type and call should only reads memory.
> +/// else return not_intrinsic.
> +Intrinsic::ID checkUnaryFloatSignature(const CallInst &I,
> +                                       Intrinsic::ID ValidIntrinsicID);
> +
> +/// \brief Identify if call has a binary float signature
> +/// It returns input intrinsic ID if call has two arguments,
> +/// arguments type and call instruction type should be floating
> +/// point type and call should only reads memory.
> +/// else return not_intrinsic.
> +Intrinsic::ID checkBinaryFloatSignature(const CallInst &I,
> +                                        Intrinsic::ID ValidIntrinsicID);
> +
> +/// \brief Returns intrinsic ID for call.
> +/// For the input call instruction it finds mapping intrinsic and returns
> +/// its intrinsic ID, in case it does not found it return not_intrinsic.
> +Intrinsic::ID getIntrinsicIDForCall(CallInst *CI, const TargetLibraryInfo *TLI);
>
>  } // llvm namespace
>
>
> Modified: llvm/trunk/lib/Transforms/Utils/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CMakeLists.txt?rev=240794&r1=240793&r2=240794&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/CMakeLists.txt (original)
> +++ llvm/trunk/lib/Transforms/Utils/CMakeLists.txt Fri Jun 26 11:57:30 2015
> @@ -37,6 +37,7 @@ add_llvm_library(LLVMTransformUtils
>    UnifyFunctionExitNodes.cpp
>    Utils.cpp
>    ValueMapper.cpp
> +  VectorUtils.cpp
>
>    ADDITIONAL_HEADER_DIRS
>    ${LLVM_MAIN_INCLUDE_DIR}/llvm/Transforms
>
> Copied: llvm/trunk/lib/Transforms/Utils/VectorUtils.cpp (from r240790, llvm/trunk/include/llvm/Transforms/Utils/VectorUtils.h)
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/VectorUtils.cpp?p2=llvm/trunk/lib/Transforms/Utils/VectorUtils.cpp&p1=llvm/trunk/include/llvm/Transforms/Utils/VectorUtils.h&r1=240790&r2=240794&rev=240794&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Transforms/Utils/VectorUtils.h (original)
> +++ llvm/trunk/lib/Transforms/Utils/VectorUtils.cpp Fri Jun 26 11:57:30 2015
> @@ -1,4 +1,4 @@
> -//===- llvm/Transforms/Utils/VectorUtils.h - Vector utilities -*- C++ -*-=====//
> +//===----------- VectorUtils.cpp - Vectorizer utility functions -----------===//
>  //
>  //                     The LLVM Compiler Infrastructure
>  //
> @@ -7,25 +7,17 @@
>  //
>  //===----------------------------------------------------------------------===//
>  //
> -// This file defines some vectorizer utilities.
> +// This file defines vectorizer utilities.
>  //
>  //===----------------------------------------------------------------------===//
>
> -#ifndef LLVM_TRANSFORMS_UTILS_VECTORUTILS_H
> -#define LLVM_TRANSFORMS_UTILS_VECTORUTILS_H
> -
> -#include "llvm/Analysis/TargetLibraryInfo.h"
> -#include "llvm/IR/IntrinsicInst.h"
> -#include "llvm/IR/Intrinsics.h"
> -
> -namespace llvm {
> +#include "llvm/Transforms/Utils/VectorUtils.h"
>
>  /// \brief Identify if the intrinsic is trivially vectorizable.
> -///
>  /// This method returns true if the intrinsic's argument types are all
>  /// scalars for the scalar form of the intrinsic and all vectors for
>  /// the vector form of the intrinsic.
> -static inline bool isTriviallyVectorizable(Intrinsic::ID ID) {
> +bool llvm::isTriviallyVectorizable(Intrinsic::ID ID) {
>    switch (ID) {
>    case Intrinsic::sqrt:
>    case Intrinsic::sin:
> @@ -59,52 +51,72 @@ static inline bool isTriviallyVectorizab
>    }
>  }
>
> -static inline bool hasVectorInstrinsicScalarOpd(Intrinsic::ID ID,
> -                                         unsigned ScalarOpdIdx) {
> +/// \brief Identifies if the intrinsic has a scalar operand. It check for
> +/// ctlz,cttz and powi special intrinsics whose argument is scalar.
> +bool llvm::hasVectorInstrinsicScalarOpd(Intrinsic::ID ID,
> +                                        unsigned ScalarOpdIdx) {
>    switch (ID) {
> -    case Intrinsic::ctlz:
> -    case Intrinsic::cttz:
> -    case Intrinsic::powi:
> -      return (ScalarOpdIdx == 1);
> -    default:
> -      return false;
> +  case Intrinsic::ctlz:
> +  case Intrinsic::cttz:
> +  case Intrinsic::powi:
> +    return (ScalarOpdIdx == 1);
> +  default:
> +    return false;
>    }
>  }
>
> -static Intrinsic::ID checkUnaryFloatSignature(const CallInst &I,
> -                                              Intrinsic::ID ValidIntrinsicID) {
> +/// \brief Check call has a unary float signature
> +/// It checks following:
> +/// a) call should have a single argument
> +/// b) argument type should be floating point type
> +/// c) call instruction type and argument type should be same
> +/// d) call should only reads memory.
> +/// If all these condition is met then return ValidIntrinsicID
> +/// else return not_intrinsic.
> +llvm::Intrinsic::ID
> +llvm::checkUnaryFloatSignature(const CallInst &I,
> +                               Intrinsic::ID ValidIntrinsicID) {
>    if (I.getNumArgOperands() != 1 ||
>        !I.getArgOperand(0)->getType()->isFloatingPointTy() ||
> -      I.getType() != I.getArgOperand(0)->getType() ||
> -      !I.onlyReadsMemory())
> +      I.getType() != I.getArgOperand(0)->getType() || !I.onlyReadsMemory())
>      return Intrinsic::not_intrinsic;
>
>    return ValidIntrinsicID;
>  }
>
> -static Intrinsic::ID checkBinaryFloatSignature(const CallInst &I,
> -                                               Intrinsic::ID ValidIntrinsicID) {
> +/// \brief Check call has a binary float signature
> +/// It checks following:
> +/// a) call should have 2 arguments.
> +/// b) arguments type should be floating point type
> +/// c) call instruction type and arguments type should be same
> +/// d) call should only reads memory.
> +/// If all these condition is met then return ValidIntrinsicID
> +/// else return not_intrinsic.
> +llvm::Intrinsic::ID
> +llvm::checkBinaryFloatSignature(const CallInst &I,
> +                                Intrinsic::ID ValidIntrinsicID) {
>    if (I.getNumArgOperands() != 2 ||
>        !I.getArgOperand(0)->getType()->isFloatingPointTy() ||
>        !I.getArgOperand(1)->getType()->isFloatingPointTy() ||
>        I.getType() != I.getArgOperand(0)->getType() ||
> -      I.getType() != I.getArgOperand(1)->getType() ||
> -      !I.onlyReadsMemory())
> +      I.getType() != I.getArgOperand(1)->getType() || !I.onlyReadsMemory())
>      return Intrinsic::not_intrinsic;
>
>    return ValidIntrinsicID;
>  }
>
> -static Intrinsic::ID
> -getIntrinsicIDForCall(CallInst *CI, const TargetLibraryInfo *TLI) {
> +/// \brief Returns intrinsic ID for call.
> +/// For the input call instruction it finds mapping intrinsic and returns
> +/// its ID, in case it does not found it return not_intrinsic.
> +llvm::Intrinsic::ID llvm::getIntrinsicIDForCall(CallInst *CI,
> +                                                const TargetLibraryInfo *TLI) {
>    // If we have an intrinsic call, check if it is trivially vectorizable.
>    if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(CI)) {
>      Intrinsic::ID ID = II->getIntrinsicID();
>      if (isTriviallyVectorizable(ID) || ID == Intrinsic::lifetime_start ||
>          ID == Intrinsic::lifetime_end || ID == Intrinsic::assume)
>        return ID;
> -    else
> -      return Intrinsic::not_intrinsic;
> +    return Intrinsic::not_intrinsic;
>    }
>
>    if (!TLI)
> @@ -199,7 +211,3 @@ getIntrinsicIDForCall(CallInst *CI, cons
>
>    return Intrinsic::not_intrinsic;
>  }
> -
> -} // llvm namespace
> -
> -#endif
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list