[llvm] r214135 - IR: Optimize size of use-list order shuffle vectors

Duncan P. N. Exon Smith dexonsmith at apple.com
Tue Jul 29 12:29:56 PDT 2014


> On 2014-Jul-29, at 10:44, Chandler Carruth <chandlerc at gmail.com> wrote:
> 
>> On Tue, Jul 29, 2014 at 10:33 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> Given that this is all still experimental, I'm not at the stage of
>> profiling this.  I'd used `SmallVector` without really thinking about
>> it, and then when I noticed the error I replaced it with something
>> strictly better.  If you feel strongly enough that this needs to be
>> reverted immediately (well, switch to `std::vector`), let me know.
>> 
>> Otherwise, it's likely to change anyway, since shuffles don't need
>> nearly this much memory (see my way-too-long-to-read llvmdev post).
> 
> If this code isn't likely to stay, and can't even be profiled, *please* stop tuning it and introducing needless abstractions. What is it even doing in tree??? =[[[[

It can be profiled.  I expect it to stay in some form, but likely be
optimized further once I get a chance to profile it and tune it.  I'm
not there yet, but I'll come back.

I (still) believe the code is better now than it would be with a
`std::vector`, but I'm willing to slot that in anyway since I'd like to
move on and won't immediately be profiling it.  Let me know if you think
that's best.

This code is in the tree because I'm developing incrementally.  While I
could develop in branch off to the side, I'd rather get early feedback.
Case in point: I thought this was an obvious improvement at the cost of
a thin abstraction, but you and David have challenged my definitions of
"obvious" and "thin".  Even if this commit stays, I've adjusted my
metrics.

> I really dislike committing "throw-away" code. It wastes reviewers time (see this thread), it wastes contributors time (see chapuni's fixes to your patches to keep the bots green) and it makes it really hard to see the actual code intended.
> 
> Either pull this out of tree, or treat it as real code and respond to the code review? =[

This wasn't throw-away code and I value the reviews.

What are you after here?




More information about the llvm-commits mailing list