[llvm] r310189 - [ADT] Add a much simpler loop to DenseMap::clear when the types are

[David Blaikie via llvm-commits]

On Sat, Aug 5, 2017 at 3:49 PM Chandler Carruth via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: chandlerc
> Date: Sat Aug  5 15:48:37 2017
> New Revision: 310189
>
> URL: http://llvm.org/viewvc/llvm-project?rev=310189&view=rev
> Log:
> [ADT] Add a much simpler loop to DenseMap::clear when the types are
> POD-like and we can just splat the empty key across memory.
>
> Sadly we can't optimize the normal loop well enough because we can't
> turn the conditional store into an unconditional store according to the
> memory model.
>
> This loop actually showed up in a profile of code that was calling clear
> as a serious source of time. =[
>
> Modified:
>     llvm/trunk/include/llvm/ADT/DenseMap.h
>
> Modified: llvm/trunk/include/llvm/ADT/DenseMap.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseMap.h?rev=310189&r1=310188&r2=310189&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/DenseMap.h (original)
> +++ llvm/trunk/include/llvm/ADT/DenseMap.h Sat Aug  5 15:48:37 2017
> @@ -107,17 +107,23 @@ public:
>      }
>
>      const KeyT EmptyKey = getEmptyKey(), TombstoneKey = getTombstoneKey();
> -    unsigned NumEntries = getNumEntries();
> -    for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P) {
> -      if (!KeyInfoT::isEqual(P->getFirst(), EmptyKey)) {
> -        if (!KeyInfoT::isEqual(P->getFirst(), TombstoneKey)) {
> -          P->getSecond().~ValueT();
> -          --NumEntries;
> -        }
> +    if (isPodLike<KeyT>::value && isPodLike<ValueT>::value) {
>

What if these conditionals ^ were rolled into the general form of the loop
in the else block? So that the dtor wouldn't be invoked for pod value
types, even if the key wasn't pod - and the key would be assigned to
EmptyKey without an isEqual check if the key is pod even if the value isn't.


> +      // Use a simpler loop when these are trivial types.
> +      for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P)
>          P->getFirst() = EmptyKey;
> +    } else {
> +      unsigned NumEntries = getNumEntries();
> +      for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P) {
> +        if (!KeyInfoT::isEqual(P->getFirst(), EmptyKey)) {
> +          if (!KeyInfoT::isEqual(P->getFirst(), TombstoneKey)) {
> +            P->getSecond().~ValueT();
> +            --NumEntries;
> +          }
> +          P->getFirst() = EmptyKey;
> +        }
>        }
> +      assert(NumEntries == 0 && "Node count imbalance!");
>      }
> -    assert(NumEntries == 0 && "Node count imbalance!");
>      setNumEntries(0);
>      setNumTombstones(0);
>    }
>
>
> _______________________________________________
> 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/20170807/11a13c4c/attachment.html>