[llvm] r264391 - Fix perfect forwarding for StringMap

[David Blaikie via llvm-commits]

On Fri, Mar 25, 2016 at 8:52 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> On Mar 25, 2016, at 8:29 AM, Mehdi Amini via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
> Here is one:
> http://lab.llvm.org:8011/builders/clang-native-arm-lnt/builds/17076/steps/build%20stage%201/logs/stdio
> I haven't had time to understand why gcc would resolve differently than
> clang.
>
>
> Also it seems MSVC is doing more move than expected:
> http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/10868/steps/ninja%20check%202/logs/stdio
> I feel this is gonna be challenging at I don't have a windows to
> reproduce...
>
>
> I think the test is relying on move-construction elision, I disabled the
> move part of the test for now. Do you have a suggestion?
>

I'd like to understand where the move construction is not being elided to
better understand the issue.

But it may be hard to test portably if the fluctuation is too great (eg: if
the difference between clang and msvc is greater than the difference
between clang with a rebucketing and clang without a rebucketing, then it's
hard to set the right bound)

But it's possible that the elision is happening somewhere we can change so
it always or never happens, to make the test more stable.


>
> --
> Mehdi
>
>
>
>
> --
> Mehdi
>
>
>
> On Mar 25, 2016, at 7:43 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
> Got a link to the failure?
> On Mar 25, 2016 7:19 AM, "Mehdi Amini" <mehdi.amini at apple.com> wrote:
>
>> Build failure on gcc bots. Not sure how to test?
>>
>> Sent from my iPhone
>>
>> On Mar 25, 2016, at 7:09 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> Test case?
>> On Mar 25, 2016 12:16 AM, "Mehdi Amini via llvm-commits" <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: mehdi_amini
>>> Date: Fri Mar 25 02:11:31 2016
>>> New Revision: 264391
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=264391&view=rev
>>> Log:
>>> Fix perfect forwarding for StringMap
>>>
>>> From: Mehdi Amini <mehdi.amini at apple.com>
>>>
>>> Modified:
>>>     llvm/trunk/include/llvm/ADT/StringMap.h
>>>
>>> Modified: llvm/trunk/include/llvm/ADT/StringMap.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringMap.h?rev=264391&r1=264390&r2=264391&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/ADT/StringMap.h (original)
>>> +++ llvm/trunk/include/llvm/ADT/StringMap.h Fri Mar 25 02:11:31 2016
>>> @@ -123,9 +123,9 @@ public:
>>>
>>>    explicit StringMapEntry(unsigned strLen)
>>>      : StringMapEntryBase(strLen), second() {}
>>> -  template <class InitTy>
>>> -  StringMapEntry(unsigned strLen, InitTy &&V)
>>> -      : StringMapEntryBase(strLen), second(std::forward<InitTy>(V)) {}
>>> +  template <typename... InitTy>
>>> +  StringMapEntry(unsigned strLen, InitTy &&... InitVals)
>>> +      : StringMapEntryBase(strLen),
>>> second(std::forward<InitTy>(InitVals)...) {}
>>>
>>>    StringRef getKey() const {
>>>      return StringRef(getKeyData(), getKeyLength());
>>> @@ -145,9 +145,9 @@ public:
>>>
>>>    /// Create a StringMapEntry for the specified key construct the value
>>> using
>>>    /// \p InitiVals.
>>> -  template <typename AllocatorTy, typename... InitTypes>
>>> +  template <typename AllocatorTy, typename... InitTy>
>>>    static StringMapEntry *Create(StringRef Key, AllocatorTy &Allocator,
>>> -                                InitTypes &&... InitVals) {
>>> +                                InitTy &&... InitVals) {
>>>      unsigned KeyLength = Key.size();
>>>
>>>      // Allocate a new item with space for the string at the end and a
>>> null
>>> @@ -160,8 +160,7 @@ public:
>>>
>>>  static_cast<StringMapEntry*>(Allocator.Allocate(AllocSize,Alignment));
>>>
>>>      // Construct the value.
>>> -    new (NewItem)
>>> -        StringMapEntry(KeyLength, std::forward<InitTypes>(InitVals)...);
>>> +    new (NewItem) StringMapEntry(KeyLength,
>>> std::forward<InitTy>(InitVals)...);
>>>
>>>      // Copy the string information.
>>>      char *StrBuffer = const_cast<char*>(NewItem->getKeyData());
>>> @@ -172,10 +171,10 @@ public:
>>>    }
>>>
>>>    /// Create - Create a StringMapEntry with normal malloc/free.
>>> -  template<typename InitType>
>>> -  static StringMapEntry *Create(StringRef Key, InitType &&InitVal) {
>>> +  template <typename... InitType>
>>> +  static StringMapEntry *Create(StringRef Key, InitType &&... InitVal) {
>>>      MallocAllocator A;
>>> -    return Create(Key, A, std::forward<InitType>(InitVal));
>>> +    return Create(Key, A, std::forward<InitType>(InitVal)...);
>>>    }
>>>
>>>    static StringMapEntry *Create(StringRef Key) {
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160325/2a04f67d/attachment.html>