Subzero: Reduce the amount of #ifdef'd code.

src/IceAssembler.h

 //===- subzero/src/IceAssembler.h - Integrated assembler --------*- C++ -*-===//
 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 //
 // Modified by the Subzero authors.
 //
 //===----------------------------------------------------------------------===//
 //
 //                        The Subzero Code Generator
@@ skipping 45 matching lines @@
     *reinterpret_cast<T *>(Contents + Position) = Value;
   }
 
   // Emit a fixup at the current location.
   void emitFixup(AssemblerFixup *Fixup) { Fixup->set_position(size()); }
 
   // Get the size of the emitted code.
   intptr_t size() const { return Cursor - Contents; }
   uintptr_t contents() const { return Contents; }
 
-// To emit an instruction to the assembler buffer, the EnsureCapacity helper
-// must be used to guarantee that the underlying data area is big enough to
-// hold the emitted instruction. Usage:
-//
-//     AssemblerBuffer buffer;
-//     AssemblerBuffer::EnsureCapacity ensured(&buffer);
-//     ... emit bytes for single instruction ...
+  // To emit an instruction to the assembler buffer, the EnsureCapacity helper
+  // must be used to guarantee that the underlying data area is big enough to
+  // hold the emitted instruction. Usage:
+  //
+  //     AssemblerBuffer buffer;
+  //     AssemblerBuffer::EnsureCapacity ensured(&buffer);
+  //     ... emit bytes for single instruction ...
 
-#ifndef NDEBUG
   class EnsureCapacity {
     EnsureCapacity(const EnsureCapacity &) = delete;
     EnsureCapacity &operator=(const EnsureCapacity &) = delete;
 
   public:
-    explicit EnsureCapacity(AssemblerBuffer *Buffer);
+    explicit EnsureCapacity(AssemblerBuffer *Buffer) : Buffer(Buffer), Gap(0) {
+      if (Buffer->cursor() >= Buffer->limit())
+        Buffer->extendCapacity();
+#ifdef NDEBUG

[Karl, 2015/06/22 15:41:06]

Why not create a boolean (inline) function in IceDefs.h, and then use

if (IsNdebug()) return;

Or, create a constexpr and use that as the test?

[John, 2015/06/22 17:13:46]

I agree with Karl on this one. Perhaps IceDefs could have something like:

constexpr bool nDebug() {
#if NDEBUG
  return true;
#else
  return false;
#endif  // NDEBUG
}

constexpr bool allowDump() {
if ALLOW_DUMP
  return true;
#else
  return false;
#endif  // ALLOW_DUMP
}

so that we could avoid these macros everywhere.

Additionally, it would be nice to have something like


class BuildTraits {
private:
  template <class Debug, class Release, bool>
  class TypePickerImpl {
  public:
    using Type = Release;
  };

  template <class Debug, class Release>
  class TypePickerImpl<Debug, Release, false> {
  public:
    using Type = Debug;
  };

public:
  static constexpr bool ReleaseBuild = nDebug();
  static constexpr bool AllowDump = allowDump();

  // ReleaseOrDebug is bad... perhaps SecondIfDebug?
  template <class Release, class Debug>
  using ReleaseOrDebug = typename
      TypePickerImpl<Debug, Release, ReleaseBuild>::Type;
};

Which means the ifs would become

if (BuildTraits::AllowDump) { /* do work */}

[Jim Stichnoth, 2015/06/23 21:42:07]

Did both, in IceDefs.h:
  constexpr bool buildAllowsAsserts() { ... }
  constexpr bool buildAllowsExtraValidation() { ... }
Also:
  constexpr bool buildAllowsDump() { ... }

+      return;
+#endif // NDEBUG
+      validate(Buffer);
+    }
     ~EnsureCapacity();
 
   private:
     AssemblerBuffer *Buffer;
     intptr_t Gap;
 
+    void validate(AssemblerBuffer *Buffer);
     intptr_t computeGap() { return Buffer->capacity() - Buffer->size(); }
   };
 
   bool HasEnsuredCapacity;
-  bool hasEnsuredCapacity() const { return HasEnsuredCapacity; }
-#else  // NDEBUG
-  class EnsureCapacity {
-    EnsureCapacity(const EnsureCapacity &) = delete;
-    EnsureCapacity &operator=(const EnsureCapacity &) = delete;
-
-  public:
-    explicit EnsureCapacity(AssemblerBuffer *Buffer) {
-      if (Buffer->cursor() >= Buffer->limit())
-        Buffer->extendCapacity();
-    }
-  };
-
-  // When building the C++ tests, assertion code is enabled. To allow
-  // asserting that the user of the assembler buffer has ensured the
-  // capacity needed for emitting, we add a dummy method in non-debug mode.
-  bool hasEnsuredCapacity() const { return true; }
+  bool hasEnsuredCapacity() const {
+#ifdef NDEBUG

[Karl, 2015/06/22 15:41:06]

same here.

[John, 2015/06/22 17:13:46]

This makes me nervous that, with NDEBUG, some compiler might complain about
unreachable code:

return true;
return HasEnsuredCapacity;

+    // Disable the actual check in non-debug mode.
+    return true;
 #endif // NDEBUG
+    return HasEnsuredCapacity;
+  }
 
   // Returns the position in the instruction stream.
   intptr_t getPosition() const { return Cursor - Contents; }
 
   // Create and track a fixup in the current function.
   AssemblerFixup *createFixup(FixupKind Kind, const Constant *Value);
 
   const FixupRefList &fixups() const { return Fixups; }
 
   void setSize(intptr_t NewSize) {
     assert(NewSize <= size());
     Cursor = Contents + NewSize;
   }
 
 private:
   // The limit is set to kMinimumGap bytes before the end of the data area.
   // This leaves enough space for the longest possible instruction and allows
   // for a single, fast space check per instruction.
-  static const intptr_t kMinimumGap = 32;
+  static constexpr intptr_t kMinimumGap = 32;
 
   uintptr_t Contents;
   uintptr_t Cursor;
   uintptr_t Limit;
   // The member variable is named Assemblr to avoid hiding the class Assembler.
   Assembler &Assemblr;
   // List of pool-allocated fixups relative to the current function.
   FixupRefList Fixups;
 
   uintptr_t cursor() const { return Cursor; }
@@ skipping 90 matching lines @@
 protected:
   // Buffer's constructor uses the Allocator, so it needs to appear after it.
   // TODO(jpp): dependencies on construction order are a nice way of shooting
   // yourself in the foot. Fix this.
   AssemblerBuffer Buffer;
 };
 
 } // end of namespace Ice
 
 #endif // SUBZERO_SRC_ICEASSEMBLER_H_