[PATCH] D31057: Make Argument::getArgNo() constant time, not O(#args)

[David Blaikie via llvm-commits]

Now that Arguments are contiguous in the Function - could you remove the
ArgNo member and compute it in O(1) by offset from the beginning of the
Arguments on the Function?

On Thu, Mar 16, 2017 at 2:56 PM Reid Kleckner via Phabricator via
llvm-commits <llvm-commits at lists.llvm.org> wrote:

> rnk created this revision.
> Herald added a subscriber: mehdi_amini.
>
> getArgNo is actually hot in LLVM, because its how we check for
> attributes on arguments:
>
>   bool Argument::hasNonNullAttr() const {
>     if (!getType()->isPointerTy()) return false;
>     if (getParent()->getAttributes().
>           hasAttribute(getArgNo()+1, Attribute::NonNull))
>       return true;
>
> It actually shows up as the 23rd hottest leaf function in a 13s sample
> of LTO of llc.
>
> This grows Argument by four bytes, but I have another pending patch to
> shrink it by removing its ilist_node base.
>
>
> https://reviews.llvm.org/D31057
>
> Files:
>   include/llvm/IR/Argument.h
>   lib/IR/Function.cpp
>
>
> Index: lib/IR/Function.cpp
> ===================================================================
> --- lib/IR/Function.cpp
> +++ lib/IR/Function.cpp
> @@ -39,27 +39,15 @@
>
>  void Argument::anchor() { }
>
> -Argument::Argument(Type *Ty, const Twine &Name)
> -    : Value(Ty, Value::ArgumentVal), Parent(nullptr) {
> +Argument::Argument(Type *Ty, const Twine &Name, unsigned ArgNo)
> +    : Value(Ty, Value::ArgumentVal), Parent(nullptr), ArgNo(ArgNo) {
>    setName(Name);
>  }
>
>  void Argument::setParent(Function *parent) {
>    Parent = parent;
>  }
>
> -unsigned Argument::getArgNo() const {
> -  const Function *F = getParent();
> -  assert(F && "Argument is not in a function");
> -
> -  Function::const_arg_iterator AI = F->arg_begin();
> -  unsigned ArgIdx = 0;
> -  for (; &*AI != this; ++AI)
> -    ++ArgIdx;
> -
> -  return ArgIdx;
> -}
> -
>  bool Argument::hasNonNullAttr() const {
>    if (!getType()->isPointerTy()) return false;
>    if (getParent()->getAttributes().
> @@ -240,7 +228,8 @@
>    for (unsigned i = 0, e = FT->getNumParams(); i != e; ++i) {
>      assert(!FT->getParamType(i)->isVoidTy() &&
>             "Cannot have void typed arguments!");
> -    ArgumentList.push_back(new Argument(FT->getParamType(i)));
> +    ArgumentList.push_back(
> +        new Argument(FT->getParamType(i), "", i));
>    }
>
>    // Clear the lazy arguments bit.
> Index: include/llvm/IR/Argument.h
> ===================================================================
> --- include/llvm/IR/Argument.h
> +++ include/llvm/IR/Argument.h
> @@ -32,21 +32,22 @@
>  class Argument : public Value, public ilist_node<Argument> {
>    virtual void anchor();
>    Function *Parent;
> +  unsigned ArgNo;
>
>    friend class SymbolTableListTraits<Argument>;
>    void setParent(Function *parent);
>
>  public:
>    /// Argument constructor.
> -  explicit Argument(Type *Ty, const Twine &Name = "");
> +  explicit Argument(Type *Ty, const Twine &Name = "", unsigned ArgNo = 0);
>
>    inline const Function *getParent() const { return Parent; }
>    inline       Function *getParent()       { return Parent; }
>
>    /// Return the index of this formal argument in its containing function.
>    ///
>    /// For example in "void foo(int a, float b)" a is 0 and b is 1.
> -  unsigned getArgNo() const;
> +  unsigned getArgNo() const { return ArgNo; }
>
>    /// Return true if this argument has the nonnull attribute. Also
> returns true
>    /// if at least one byte is known to be dereferenceable and the pointer
> is in
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170320/850f4c37/attachment.html>