[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