[cfe-commits] r149973 - /cfe/trunk/lib/CodeGen/BackendUtil.cpp

[David Blaikie]

On Tue, Feb 7, 2012 at 11:36 AM, Bill Wendling <isanbard at gmail.com> wrote:
> On Feb 7, 2012, at 10:22 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Tue, Feb 7, 2012 at 1:05 AM, Bill Wendling <isanbard at gmail.com> wrote:
>>> Author: void
>>> Date: Tue Feb  7 03:05:34 2012
>>> New Revision: 149973
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=149973&view=rev
>>> Log:
>>> Reserve a moderate amount of space for the back-end arguments.
>>>
>>> Modified:
>>>    cfe/trunk/lib/CodeGen/BackendUtil.cpp
>>>
>>> Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=149973&r1=149972&r2=149973&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Tue Feb  7 03:05:34 2012
>>> @@ -247,6 +247,7 @@
>>>   }
>>>
>>>   std::vector<const char *> BackendArgs;
>>> +  BackendArgs.reserve(16);
>>
>> Would a SmallVector be a better fit, then? (see attached patches - I
>> also went down a small rabbit hole when I discovered that
>> ParseCommandLineOptions couldn't be passed a buffer of const char*s -
>> by fixing the const down in LLVM's command line support I was able to
>> enable this & remove a few const_casts from clang's various drivers
>> too. (another thing to do down there might be to change CommandLine's
>> internal buffering to SmallVector rather than std::vector (see newArgv
>> in CommandLine.cpp:479 - though I only discovered this when I realized
>> I couldn't change line 486 to use data() because vector::data()
>> doesn't exist in C++98))
>>
> Hi David,
>
> I thought about SmallVector as well, but I didn't want to make anything but cosmetic changes (which would hopefully result in compile time improvements).

Fair enough - I'd be surprised if the perf difference here was
observable (but it'd be great if it does improve things) are you
actually trying to observe an improvement (or observed a profile where
this was costly before your change) here or just saw it & figured it
couldn't hurt?

> These patches look good to me. (Yay! for getting rid of const_casts!) Please go ahead and apply them. Thanks! :-)

Thanks for the review - r149999 and r150000 (winner!)

- David