HomePhabricator

Strength-reduce SmallVectors to arrays. NFCI.

Authored by bkramer on Fri, Aug 28, 12:14 PM.

Description

Strength-reduce SmallVectors to arrays. NFCI.

Details

Committed
bkramerFri, Aug 28, 12:14 PM
Parents
rG52cc97a0db2d: [CodeGenPrepare] Zap the argument of llvm.assume when deleting it
Branches
Unknown
Tags
Unknown

Event Timeline

bondhugula added inline comments.
/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
3159

This pattern can be problematic from a debugging standpoint. If you get the number of the std::array template higher, the compiler won't complain (no error / no warning AFAIR). Most of the methods that take such an array while implicitly converting them to an ArrayRef would have trailing default/null initialized values (or garbage values?) will continue to work and a crash/corruption would show up much later. Because the methods that take these ArrayRef's will only typically assert on its size - not on individual elements. However, if one uses SmallVector, a higher than necessary value on the template would still yield the right size and a wrong number of elements would get asserted on in the method it was passed to. OTOH, with std::array, if you initialize the list with fewer values while having the right size on template, we are once again in trouble.

In both cases, you have the elements on the stack. Accessing an std::array is faster I guess due to the lack of any indirection. I'm not sure though why the compiler doesn't warn if the initializer list has fewer values.

bkramer added inline comments.Mon, Aug 31, 2:53 AM
/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
3159

Yeah, this is C compatibility leaking into newer C++ features. The remainder is filled with default-constructed values.

An alternative is using a C-style array that autodetects its size.

const StoreInst *FreeStores[] = {PStore, QStore};

Do you think that's better?

bondhugula added inline comments.Thu, Sep 3, 10:06 AM
/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
3159

This looks too Cish surrounded by C++ but certainly the best of all! I have no additional insights here on what may be better! :-) But getting the count wrong on std::array is going to be a quite common thing --- also during code updates. I'd personally just prefer SmallVector in non-performance critical code - the extra load should be fine. It's also likely detected as invariant on the loop iterating the SmallVector.