[PATCH] Add infrastructure for support of multiple memory constraints.

[hfinkel at anl.gov]

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6598
@@ +6597,3 @@
+        if (ConstraintID == InlineAsm::Constraint_Unknown)
+          llvm_unreachable("Failed to convert memory constraint code to constraint id.");
+
----------------
dsanders wrote:
> hfinkel wrote:
> > Line too long. Also, why not write?
> > 
> >   assert(ConstraintID != InlineAsm::Constraint_Unknown &&
> >          "Failed to convert memory constraint code to constraint id.");
> > 
> > (same below too)
> If I use an assertion, then builds without assertions enabled will act as if nothing was wrong and try to continue. For most targets, this will mean that they die in SelectInlineAsmMemoryOperand() when it returns true, but a couple targets (Mips and SystemZ) will never notice and generate code. I could change Mips and SystemZ but there's also out-of-tree targets which might do the same thing. Overall, I think it's better to fail here for all builds.
I used to think this too (until very recently), but it's not true. llvm_unreachable is really just like assert; the implementation looks like this:

  /// Marks that the current location is not supposed to be reachable.
  /// In !NDEBUG builds, prints the message and location info to stderr.
  /// In NDEBUG builds, becomes an optimizer hint that the current location
  /// is not supposed to be reachable.  On compilers that don't support
  /// such hints, prints a reduced message instead.
  ///
  /// Use this instead of assert(0).  It conveys intent more clearly and
  /// allows compilers to omit some unnecessary code.
  #ifndef NDEBUG
  #define llvm_unreachable(msg) \
    ::llvm::llvm_unreachable_internal(msg, __FILE__, __LINE__)
  #elif defined(LLVM_BUILTIN_UNREACHABLE)
  #define llvm_unreachable(msg) LLVM_BUILTIN_UNREACHABLE
  #else
  #define llvm_unreachable(msg) ::llvm::llvm_unreachable_internal()
  #endif

where LLVM_BUILTIN_UNREACHABLE in defined as:

  /// LLVM_BUILTIN_UNREACHABLE - On compilers which support it, expands
  /// to an expression which states that it is undefined behavior for the
  /// compiler to reach this point.  Otherwise is not defined.
  #if __has_builtin(__builtin_unreachable) || LLVM_GNUC_PREREQ(4, 5, 0)
  # define LLVM_BUILTIN_UNREACHABLE __builtin_unreachable()
  #elif defined(_MSC_VER)
  # define LLVM_BUILTIN_UNREACHABLE __assume(false)
  #endif

And so, on recent compilers, this *really* is just like an assert, in fact, it is even stronger than that (providing explicit information to the optimizer than the condition is impossible).

Thus, please just use an assert.

http://reviews.llvm.org/D8171

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/