Introduce the notion of function addresses in Subzero.

src/IceConverter.cpp

Index: src/IceConverter.cpp
diff --git a/src/IceConverter.cpp b/src/IceConverter.cpp
index 380f1a0dd88be21afc73e5fea42567ac72af24d9..c42eefa17caee3e5c52e9fc3849664b046aa30f0 100644
--- a/src/IceConverter.cpp
+++ b/src/IceConverter.cpp
@@ -52,11 +52,15 @@ class LLVM2ICEConverter {
   LLVM2ICEConverter &operator=(const LLVM2ICEConverter &) = delete;
 
 public:
-  LLVM2ICEConverter(Ice::GlobalContext *Ctx, LLVMContext &LLVMContext)
-      : Ctx(Ctx), TypeConverter(LLVMContext) {}
+  LLVM2ICEConverter(Ice::Converter &Converter)
+      : Converter(Converter), Ctx(Converter.getContext()),
+        TypeConverter(Converter.getModule()->getContext()) {}
+
+public:

[jvoung (off chromium), 2014/10/10 01:47:27]

already in the public: section?

[Karl, 2014/10/10 20:17:29]

Removed.

+  Ice::Converter &getConverter() { return Converter; }

[jvoung (off chromium), 2014/10/10 01:47:28]

const ?

[Karl, 2014/10/10 20:17:30]

Done.

 
 protected:
-  // Data
+  Ice::Converter &Converter;
   Ice::GlobalContext *Ctx;
   const Ice::TypeConverter TypeConverter;
 };
@@ -72,8 +76,8 @@ class LLVM2ICEFunctionConverter : LLVM2ICEConverter {
   operator=(const LLVM2ICEFunctionConverter &) = delete;
 
 public:
-  LLVM2ICEFunctionConverter(Ice::GlobalContext *Ctx, LLVMContext &LLVMContext)
-      : LLVM2ICEConverter(Ctx, LLVMContext), Func(NULL) {}
+  LLVM2ICEFunctionConverter(Ice::Converter &Converter)
+      : LLVM2ICEConverter(Converter), Func(NULL) {}
 
   // Caller is expected to delete the returned Ice::Cfg object.
   Ice::Cfg *convertFunction(const Function *F) {
@@ -636,48 +640,35 @@ class LLVM2ICEGlobalsConverter : public LLVM2ICEConverter {
   operator-(const LLVM2ICEGlobalsConverter &) = delete;
 
 public:
-  LLVM2ICEGlobalsConverter(Ice::GlobalContext *Ctx, LLVMContext &LLVMContext)
-      : LLVM2ICEConverter(Ctx, LLVMContext) {}
-
-  ~LLVM2ICEGlobalsConverter() { DeleteContainerSeconds(GlobalVarAddressMap); }
+  LLVM2ICEGlobalsConverter(Ice::Converter &Converter)
+      : LLVM2ICEConverter(Converter) {}
 
   /// Converts global variables, and their initializers into ICE global
   /// addresses, for module Mod. Puts corresponding converted global
-  /// addresses into GlobalAddresses.
-  void convertGlobalsToIce(Module *Mod,
-                           Ice::Translator::GlobalAddressList &GlobalAddresses);
+  /// addresses into GlobalVariables.
+  void
+  convertGlobalsToIce(Module *Mod,
+                      Ice::Translator::GlobalVariableListType &GlobalVariables);
 
 private:
-  typedef std::map<const GlobalVariable *, Ice::GlobalAddress *>
-      GlobalVarAddressMapType;
-  // Map from global variables to their corresponding global address.
-  GlobalVarAddressMapType GlobalVarAddressMap;
-
   // Adds the Initializer to the list of initializers for Global address.
-  void addGlobalInitializer(Ice::GlobalAddress &Global,
+  void addGlobalInitializer(Ice::GlobalVariable &Global,
                             const Constant *Initializer) {
     const bool HasOffset = false;
-    const Ice::GlobalAddress::RelocOffsetType Offset = 0;
+    const Ice::GlobalVariable::RelocOffsetType Offset = 0;
     addGlobalInitializer(Global, Initializer, HasOffset, Offset);
   }
 
   // Adds Initializer to the list of initializers for Global
   // address. HasOffset is true only if Initializer is a relocation
   // initializer and Offset should be added to the relocation.
-  void addGlobalInitializer(Ice::GlobalAddress &Global,
+  void addGlobalInitializer(Ice::GlobalVariable &Global,
                             const Constant *Initializer, bool HasOffset,
-                            Ice::GlobalAddress::RelocOffsetType Offset);
-
-  // Returns the global address associated with global variable GV.
-  Ice::GlobalAddress *getGlobalVarAddress(const GlobalVariable *GV) {
-    if (GlobalVarAddressMap.find(GV) == GlobalVarAddressMap.end())
-      GlobalVarAddressMap[GV] = new Ice::GlobalAddress();
-    return GlobalVarAddressMap[GV];
-  }
+                            Ice::GlobalVariable::RelocOffsetType Offset);
 
   // Converts the given constant C to the corresponding integer
   // literal it contains.
-  Ice::GlobalAddress::RelocOffsetType
+  Ice::GlobalVariable::RelocOffsetType
   getIntegerLiteralConstant(const Value *C) {
     const auto CI = dyn_cast<ConstantInt>(C);
     if (CI && CI->getType()->isIntegerTy(32))
@@ -692,7 +683,7 @@ private:
 };
 
 void LLVM2ICEGlobalsConverter::convertGlobalsToIce(
-    Module *Mod, Ice::Translator::GlobalAddressList &GlobalAddresses) {
+    Module *Mod, Ice::Translator::GlobalVariableListType &GlobalVariables) {
   for (Module::const_global_iterator I = Mod->global_begin(),
                                      E = Mod->global_end();
        I != E; ++I) {
@@ -709,13 +700,10 @@ void LLVM2ICEGlobalsConverter::convertGlobalsToIce(
       report_fatal_error(StrBuf.str());
     }
 
-    Ice::GlobalAddress *Addr = getGlobalVarAddress(GV);
-    GlobalAddresses.push_back(Addr);
-    Addr->setAlignment(GV->getAlignment());
-    Addr->setIsConstant(GV->isConstant());
-    // Note: We allow external for cross tests.
-    Addr->setIsInternal(!GV->isExternallyInitialized());
-    Addr->setName(Name);
+    Ice::GlobalAddress *Addr = getConverter().getGlobalAddress(GV);
+    assert(llvm::isa<Ice::GlobalVariable>(Addr));

[jvoung (off chromium), 2014/10/10 01:47:28]

sometimes llvm:: is used, but sometimes not?

[Karl, 2014/10/10 20:17:29]

Assuming we should always use on GlobalVariable, since it is in both namespaces.

Note that I have renamed Ice::GlobalVariable to Ice::VariableDeclaration,
avoiding namespace clashes.

+    auto VarAddr = llvm::cast<Ice::GlobalVariable>(Addr);

[jvoung (off chromium), 2014/10/10 01:47:28]

cast<> already asserts isa<>, so the earlier assert for isa<> is redundant.

[Karl, 2014/10/10 20:17:30]

Removing redundancy.

+    GlobalVariables.push_back(VarAddr);
 
     const Constant *Initializer = GV->getInitializer();
     if (const auto CompoundInit = dyn_cast<ConstantStruct>(Initializer)) {
@@ -723,24 +711,24 @@ void LLVM2ICEGlobalsConverter::convertGlobalsToIce(
                                              E = CompoundInit->op_end();
            I != E; ++I) {
         if (const auto Init = dyn_cast<Constant>(I)) {
-          addGlobalInitializer(*Addr, Init);
+          addGlobalInitializer(*VarAddr, Init);
         }
       }
     } else {
-      addGlobalInitializer(*Addr, Initializer);
+      addGlobalInitializer(*VarAddr, Initializer);
     }
   }
 }
 
 void LLVM2ICEGlobalsConverter::addGlobalInitializer(
-    Ice::GlobalAddress &Global, const Constant *Initializer, bool HasOffset,
-    Ice::GlobalAddress::RelocOffsetType Offset) {
+    Ice::GlobalVariable &Global, const Constant *Initializer, bool HasOffset,
+    Ice::GlobalVariable::RelocOffsetType Offset) {
   assert(HasOffset || Offset == 0);
 
   if (const auto CDA = dyn_cast<ConstantDataArray>(Initializer)) {
     assert(!HasOffset && isa<IntegerType>(CDA->getElementType()) &&
            (cast<IntegerType>(CDA->getElementType())->getBitWidth() == 8));
-    Global.addInitializer(new Ice::GlobalAddress::DataInitializer(
+    Global.addInitializer(new Ice::GlobalVariable::DataInitializer(
         CDA->getRawDataValues().data(), CDA->getNumElements()));
     return;
   }
@@ -750,7 +738,7 @@ void LLVM2ICEGlobalsConverter::addGlobalInitializer(
       assert(!HasOffset && isa<IntegerType>(AT->getElementType()) &&
              (cast<IntegerType>(AT->getElementType())->getBitWidth() == 8));
       Global.addInitializer(
-          new Ice::GlobalAddress::ZeroInitializer(AT->getNumElements()));
+          new Ice::GlobalVariable::ZeroInitializer(AT->getNumElements()));
     } else {
       llvm_unreachable("Unhandled constant aggregate zero type");
     }
@@ -769,18 +757,17 @@ void LLVM2ICEGlobalsConverter::addGlobalInitializer(
              TypeConverter.getIcePointerType());
       const auto GV = dyn_cast<GlobalValue>(Exp->getOperand(0));
       assert(GV);
-      if (const auto Fcn = dyn_cast<Function>(GV)) {
-        Ice::GlobalAddress::RelocationAddress Addr(Fcn);
-        Global.addInitializer(
-            new Ice::GlobalAddress::RelocInitializer(Addr, Offset));
-        return;
-      } else if (const auto Var = dyn_cast<GlobalVariable>(GV)) {
-        Ice::GlobalAddress::RelocationAddress Addr(getGlobalVarAddress(Var));
-        Global.addInitializer(
-            new Ice::GlobalAddress::RelocInitializer(Addr, Offset));
+      const Ice::GlobalAddress *Addr = getConverter().getGlobalAddress(GV);
+      if (Addr == NULL) {

[jvoung (off chromium), 2014/10/10 01:47:28]

nullptr

[Karl, 2014/10/10 20:17:30]

Replaced all occurrences.

+        std::string Buffer;
+        raw_string_ostream StrBuf(Buffer);
+        StrBuf << "Can't find global address for: " << GV;
+        report_fatal_error(StrBuf.str());
         return;
       }
-      break;
+      Global.addInitializer(
+          new Ice::GlobalVariable::RelocInitializer(Addr, Offset));
+      return;
     }
     default:
       break;
@@ -799,22 +786,63 @@ namespace Ice {
 
 void Converter::convertToIce() {
   TimerMarker T(TimerStack::TT_convertToIce, Ctx);
-  nameUnnamedGlobalAddresses(Mod);
+  nameUnnamedGlobalVariables(Mod);
   nameUnnamedFunctions(Mod);
+  installGlobalAddresses(Mod);
   convertGlobals(Mod);
   convertFunctions();
 }
 
+GlobalAddress *Converter::getGlobalAddress(const llvm::GlobalValue *V) {
+  GlobalAddressMapType::const_iterator Pos = GlobalAddressMap.find(V);
+  if (Pos == GlobalAddressMap.end())
+    return nullptr;
+  return Pos->second;
+}
+
+void Converter::installGlobalAddresses(Module *Mod) {
+  const TypeConverter Converter(Mod->getContext());
+  // Install function addresses.
+  for (llvm::Function &Func : *Mod) {

[jvoung (off chromium), 2014/10/10 01:47:28]

could this be const ?

[Karl, 2014/10/10 20:17:29]

Done.

+    FuncSigType Signature;
+    llvm::FunctionType *FuncType = Func.getFunctionType();
+    Signature.setReturnType(
+        Converter.convertToIceType(FuncType->getReturnType()));
+    for (size_t I = 0; I < FuncType->getNumParams(); ++I) {
+      Signature.appendArgType(
+          Converter.convertToIceType(FuncType->getParamType(I)));
+    }
+    Ice::Function *IceFunc = Ice::Function::create(
+        Ctx, Signature, Func.getCallingConv(), Func.getLinkage(), Func.empty());
+    IceFunc->setName(Func.getName());
+    GlobalAddressMap[&Func] = IceFunc;
+  }
+  // Install global variable addresses.
+  for (Module::const_global_iterator I = Mod->global_begin(),
+                                     E = Mod->global_end();
+       I != E; ++I) {
+    const auto GV = dyn_cast<llvm::GlobalVariable>(I);

[jvoung (off chromium), 2014/10/10 01:47:28]

could just cast<> to assert that GV is really a GlobalVariable (instead of the
assert right after this?)

[jvoung (off chromium), 2014/10/10 01:47:28]

In some places plain "GlobalVariable" is used for "llvm::GlobalVariable", while
only "Ice::GlobalVariable" has the "Ice::". It might be better to just have
"llvm::GlobalVariable" like this instance? WDYT?

[Karl, 2014/10/10 20:17:30]

Fixed use of cast. Using explicit namespace prefix for GlobalVariable, since it
can appear in both llvm and Ice.

Note: This is no longer an issue since I am now using Ice::VariableDeclaration
instead of Ice::GlobalVariable.

+    assert(GV);
+    Ice::GlobalVariable *Vbl = Ice::GlobalVariable::create(Ctx);
+    Vbl->setName(GV->getName());
+    Vbl->setAlignment(GV->getAlignment());
+    Vbl->setIsConstant(GV->isConstant());
+    // Note: We allow external for cross tests.
+    Vbl->setIsInternal(!GV->isExternallyInitialized());

[jvoung (off chromium), 2014/10/10 01:47:27]

Was there a commandline flag to check?

[Karl, 2014/10/10 20:17:30]

Yes.

Will do in a separate CL, since many lit tests will require the addition of the
flag. Adding TODO.

+    GlobalAddressMap[GV] = Vbl;
+  }
+}
+
 void Converter::convertGlobals(Module *Mod) {
-  LLVM2ICEGlobalsConverter GlobalsConverter(Ctx, Mod->getContext());
-  Translator::GlobalAddressList GlobalAddresses;
-  GlobalsConverter.convertGlobalsToIce(Mod, GlobalAddresses);
-  lowerGlobals(GlobalAddresses);
+  LLVM2ICEGlobalsConverter GlobalsConverter(*this);
+  Translator::GlobalVariableListType GlobalVariables;
+  GlobalsConverter.convertGlobalsToIce(Mod, GlobalVariables);
+  lowerGlobals(GlobalVariables);
 }
 
 void Converter::convertFunctions() {
   TimerStackIdT StackID = GlobalContext::TSK_Funcs;
-  for (const Function &I : *Mod) {
+  for (const llvm::Function &I : *Mod) {
     if (I.empty())
       continue;
 
@@ -823,7 +851,7 @@ void Converter::convertFunctions() {
       TimerID = Ctx->getTimerID(StackID, I.getName());
       Ctx->pushTimer(TimerID, StackID);
     }
-    LLVM2ICEFunctionConverter FunctionConverter(Ctx, Mod->getContext());
+    LLVM2ICEFunctionConverter FunctionConverter(*this);
 
     Cfg *Fcn = FunctionConverter.convertFunction(&I);
     translateFcn(Fcn);