Sanitizers.h - remove MathExtras.h include dependency

[RKSimon]

Summary

We include the MathExtras.h header purely for the countPopulation() method - by moving this into Sanitizers.cpp we can remove the use of this costly header.

AFAICT we only ever use isPowerOf2() / countPopulation() insider asserts (see the CGExpr.cpp change which I noticed by altering their access to private) so this shouldn't have any performance effects on production code.

[RKSimon]

clang/lib/CodeGen/CGExpr.cpp

3129

I'm happy to revert this line, its purely to demonstrate that the methods are just used in asserts.

[pgousseau]

LGTM!

clang/include/clang/Basic/Sanitizers.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_SANITIZERS_H
 #define LLVM_CLANG_BASIC_SANITIZERS_H
 
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/MathExtras.h"
 #include "llvm/Transforms/Instrumentation/AddressSanitizerOptions.h"
 #include <cassert>
 #include <cstdint>
 
 namespace llvm {
 class hash_code;
 }
 
public:
   static constexpr SanitizerMask bitPosToMask(const unsigned Pos) {
     uint64_t mask1 = (Pos < kNumBitElem) ? 1ULL << (Pos % kNumBitElem) : 0;
     uint64_t mask2 = (Pos >= kNumBitElem && Pos < (kNumBitElem * 2))
                          ? 1ULL << (Pos % kNumBitElem)
                          : 0;
     return SanitizerMask(mask1, mask2);
   }
 
-  unsigned countPopulation() const {
-    unsigned total = 0;
-    for (const auto &Val : maskLoToHigh)
-      total += llvm::countPopulation(Val);
-    return total;
-  }
+  unsigned countPopulation() const;
 
   void flipAllBits() {
     for (auto &Val : maskLoToHigh)
       Val = ~Val;
   }
 
   bool isPowerOf2() const {
     return countPopulation() == 1;

clang/lib/Basic/Sanitizers.cpp

 //===- Sanitizers.cpp - C Language Family Language Options ----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines the classes from Sanitizers.h
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Basic/Sanitizers.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/MathExtras.h"
 
 using namespace clang;
 
 // Once LLVM switches to C++17, the constexpr variables can be inline and we
 // won't need this.
 #define SANITIZER(NAME, ID) constexpr SanitizerMask SanitizerKind::ID;
 #define SANITIZER_GROUP(NAME, ID, ALIAS)                                       \
   constexpr SanitizerMask SanitizerKind::ID;                                   \
#include "clang/Basic/Sanitizers.def"
   return Kinds;
 }
 
 llvm::hash_code SanitizerMask::hash_value() const {
   return llvm::hash_combine_range(&maskLoToHigh[0], &maskLoToHigh[kNumElem]);
 }
 
 namespace clang {
+unsigned SanitizerMask::countPopulation() const {
+  unsigned total = 0;
+  for (const auto &Val : maskLoToHigh)
+    total += llvm::countPopulation(Val);
+  return total;
+}
+
 llvm::hash_code hash_value(const clang::SanitizerMask &Arg) {
   return Arg.hash_value();
 }
 
 StringRef AsanDtorKindToString(llvm::AsanDtorKind kind) {
   switch (kind) {
   case llvm::AsanDtorKind::None:
     return "none";

clang/lib/CodeGen/CGExpr.cpp

enum class CheckRecoverableKind {
   /// non-fatal handlers for this check.
   Recoverable,
   /// Runtime conditionally aborts, always need to support recovery.
   AlwaysRecoverable
 };
 }
 
 static CheckRecoverableKind getRecoverableKind(SanitizerMask Kind) {
-  assert(Kind.countPopulation() == 1);
+  assert(Kind.isPowerOf2());
   if (Kind == SanitizerKind::Function || Kind == SanitizerKind::Vptr)
     return CheckRecoverableKind::AlwaysRecoverable;
   else if (Kind == SanitizerKind::Return || Kind == SanitizerKind::Unreachable)
     return CheckRecoverableKind::Unrecoverable;
   else
     return CheckRecoverableKind::Recoverable;
 }