Cache common constants before lowering.

src/IceGlobalContext.cpp

Index: src/IceGlobalContext.cpp
diff --git a/src/IceGlobalContext.cpp b/src/IceGlobalContext.cpp
index 505ce497b2514090cecacf7dd14597d2caf07c3c..a29d932c35be3a94deb57f70589cd4c366f13f33 100644
--- a/src/IceGlobalContext.cpp
+++ b/src/IceGlobalContext.cpp
@@ -25,6 +25,7 @@
 #include "IceOperand.h"
 #include "IceTargetLowering.h"
 #include "IceTimerTree.h"
+#include "IceTypes.def"
 #include "IceTypes.h"
 
 #ifdef __clang__
@@ -291,6 +292,18 @@ GlobalContext::GlobalContext(Ostream *OsDump, Ostream *OsEmit, Ostream *OsError,
   case FT_Iasm:
     break;
   }
+// Cache up front common constants.
+#define X(tag, sizeLog2, align, elts, elty, str, rcstr)                        \
+  ConstZeroForType[IceType_##tag] = installConstantZero(IceType_##tag);
+  ICETYPE_TABLE;
+#undef X
+  ConstantTrue = installConstantInt1(1);
+  // Define runtime helper functions.
+  {

[Jim Stichnoth, 2016/03/17 00:14:52]

Is this set of braces necessary?

[Karl, 2016/03/17 16:40:53]

Removed.

+#define X(Tag, Name) RuntimeHelperFunc[H_##Tag] = getConstantExternSym(Name);
+    RUNTIME_HELPER_FUNCTIONS_TABLE
+#undef X
+  }
 
   TargetLowering::staticInit(this);
 }
@@ -657,24 +670,24 @@ Constant *GlobalContext::getConstantInt(Type Ty, int64_t Value) {
   return nullptr;
 }
 
-Constant *GlobalContext::getConstantInt1(int8_t ConstantInt1) {
+Constant *GlobalContext::installConstantInt1(int8_t ConstantInt1) {

[Jim Stichnoth, 2016/03/17 00:14:52]

Bikeshed time.  I kinda don't like the "install" prefix, because it implies that
each call is actively installing (a new copy of) the constant somewhere.

(One could make a similar, perhaps weaker, argument about the "get" prefix
implying a read-only operation, but there's a lot of legacy and familiarity with
that prefix, so...)

What do you think about something like getConstantInt1Internal() ?

[Karl, 2016/03/17 16:40:53]

Done.

   ConstantInt1 &= INT8_C(1);
   return getConstPool()->Integers1.getOrAdd(this, ConstantInt1);
 }
 
-Constant *GlobalContext::getConstantInt8(int8_t ConstantInt8) {
+Constant *GlobalContext::installConstantInt8(int8_t ConstantInt8) {
   return getConstPool()->Integers8.getOrAdd(this, ConstantInt8);
 }
 
-Constant *GlobalContext::getConstantInt16(int16_t ConstantInt16) {
+Constant *GlobalContext::installConstantInt16(int16_t ConstantInt16) {
   return getConstPool()->Integers16.getOrAdd(this, ConstantInt16);
 }
 
-Constant *GlobalContext::getConstantInt32(int32_t ConstantInt32) {
+Constant *GlobalContext::installConstantInt32(int32_t ConstantInt32) {
   return getConstPool()->Integers32.getOrAdd(this, ConstantInt32);
 }
 
-Constant *GlobalContext::getConstantInt64(int64_t ConstantInt64) {
+Constant *GlobalContext::installConstantInt64(int64_t ConstantInt64) {
   return getConstPool()->Integers64.getOrAdd(this, ConstantInt64);
 }
 
@@ -711,39 +724,32 @@ Constant *GlobalContext::getConstantUndef(Type Ty) {
 }
 
 // All locking is done by the getConstant*() target function.
-Constant *GlobalContext::getConstantZero(Type Ty) {
+Constant *GlobalContext::installConstantZero(Type Ty) {
   switch (Ty) {
   case IceType_i1:
-    return getConstantInt1(0);
+    return installConstantInt1(0);
+    break;

[Jim Stichnoth, 2016/03/17 00:14:52]

You don't need all these "break" statements after a return.

[Karl, 2016/03/17 16:40:53]

Done.

   case IceType_i8:
-    return getConstantInt8(0);
+    return installConstantInt8(0);
+    break;
   case IceType_i16:
-    return getConstantInt16(0);
+    return installConstantInt16(0);
+    break;
   case IceType_i32:
-    return getConstantInt32(0);
+    return installConstantInt32(0);
+    break;
   case IceType_i64:
-    return getConstantInt64(0);
+    return installConstantInt64(0);
+    break;
   case IceType_f32:
     return getConstantFloat(0);
+    break;
   case IceType_f64:
     return getConstantDouble(0);
-  case IceType_v4i1:
-  case IceType_v8i1:
-  case IceType_v16i1:
-  case IceType_v16i8:
-  case IceType_v8i16:
-  case IceType_v4i32:
-  case IceType_v4f32: {
-    IceString Str;
-    llvm::raw_string_ostream BaseOS(Str);
-    BaseOS << "Unsupported constant type: " << Ty;
-    llvm_unreachable(BaseOS.str().c_str());
-  } break;
-  case IceType_void:
-  case IceType_NUM:
     break;
+  default:
+    return nullptr;

[Jim Stichnoth, 2016/03/17 00:14:52]

I kind of like the previous behavior where it gives an error for an
inappropriate type.

What do you think of adding a second parameter to this method, to enable hard
fail on invalid type?

Constant *GlobalContext::installConstantZero(Type Ty, bool AllowInvalidType =
false) {
  ...
  default:
    if (AllowInvalidType)
      return nullptr;
    llvm::report_fatal_error("Unknown type");
}

[John, 2016/03/17 14:31:33]

I also like the previous behavior better, but I am not so sure about the extra
parameter. How about doing something like

Constant *GlobalContext::installConstantZero(Type Ty) {
  // current implementation.
}

Constant *GlobalContext::getConstantZero(Type Ty){
  auto *Zero = installConstantZero(Ty);
  if (Zero == nullptr) {
    report_fatal_error();
  }
  return Zero;
}

?

[Karl, 2016/03/17 16:40:53]

Followed John's advice.

   }
-  llvm_unreachable("Unknown type");
 }
 
 ConstantList GlobalContext::getConstantPool(Type Ty) {