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

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 29 10:49:10 PDT 2017


rnk added a comment.

In https://reviews.llvm.org/D31057#711679, @sabuasal wrote:

> Hi, 
>  This seems like a nice optimization.


Thanks!

> I was just wondering if it is a good idea to silently push this with a default argument for ArgNo. there is other locations in LLVM where an Argument object is constructed with the default parameter without realizing that this will break their getArgNo.
> 
> for example:
>  git grep -n  "new Argument("
>  lib/AsmParser/LLParser.cpp:2533:    FwdVal = new Argument(Ty, Name);
>  lib/AsmParser/LLParser.cpp:2573:    FwdVal = new Argument(Ty);
>  lib/Bitcode/Reader/ValueList.cpp:116:  Value *V = new Argument(Ty);

These arguments are all only ever used to forward declare a Value. They never have a parent function. `getArgNo()` will return 0, but if `getParent()` returns null, what does that really mean?

After searching for calls to `new Argument` across clang and LLVM, I realized that everyone uses the code in Function to construct the argument list. Perhaps one nice side effect of eliminating this field and computing `getArgNo()` from `getParent()` is that it would be impossible to get the argument number of an unparented argument. That seems like a better argument for doing it than the memory usage of this field.


Repository:
  rL LLVM

https://reviews.llvm.org/D31057





More information about the llvm-commits mailing list