Introduce the notion of function addresses in Subzero.

src/IceConverter.cpp

 //===- subzero/src/IceConverter.cpp - Converts LLVM to Ice  ---------------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the LLVM to ICE converter.
@@ skipping 16 matching lines @@
 #include "IceConverter.h"
 #include "IceDefs.h"
 #include "IceGlobalContext.h"
 #include "IceGlobalInits.h"
 #include "IceInst.h"
 #include "IceOperand.h"
 #include "IceTargetLowering.h"
 #include "IceTypes.h"
 #include "IceTypeConverter.h"
 
 using namespace llvm;

[Jim Stichnoth, 2014/10/10 13:15:00]

A number of "llvm::" prefixes crept in, should they be removed?

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

There are several comments around the topic of namespace prefixes, each having
(somewhat conflicting) suggestions.

One philosophy I see is not wanting to add the prefix when possible. The other
is to always refer to a name consistently. Complicating this is that several
names are in both namespace llvm and Ice.

After talking with Jim, I've concluded that code should at most, use one
namespace, so that there isn't hidden name clashes. For the moment,
I have fixed cases which allow multiple namespaces. Rather I have put this off
to a separate CL.

 
 namespace {
 
 // Debugging helper
 template <typename T> static std::string LLVMObjectAsString(const T *O) {
   std::string Dump;
   raw_string_ostream Stream(Dump);
   O->print(Stream);
   return Stream.str();
 }
 
 // Base class for converting LLVM to ICE.
 class LLVM2ICEConverter {
   LLVM2ICEConverter(const LLVM2ICEConverter &) = delete;
   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;
 };
 
 // Converter from LLVM functions to ICE. The entry point is the
 // convertFunction method.
 //
 // Note: this currently assumes that the given IR was verified to be
 // valid PNaCl bitcode. Otherwise, the behavior is undefined.
 class LLVM2ICEFunctionConverter : LLVM2ICEConverter {
   LLVM2ICEFunctionConverter(const LLVM2ICEFunctionConverter &) = delete;
   LLVM2ICEFunctionConverter &
   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) {
     VarMap.clear();
     NodeMap.clear();
     Func = new Ice::Cfg(Ctx);
     Func->setFunctionName(F->getName());
     Func->setReturnType(convertToIceType(F->getReturnType()));
     Func->setInternal(F->hasInternalLinkage());
     Ice::TimerMarker T(Ice::TimerStack::TT_llvmConvert, Func);
@@ skipping 542 matching lines @@
 // convertGlobalsToIce method.
 //
 // Note: this currently assumes that the given IR was verified to be
 // valid PNaCl bitcode. Othewise, the behavior is undefined.
 class LLVM2ICEGlobalsConverter : public LLVM2ICEConverter {
   LLVM2ICEGlobalsConverter(const LLVM2ICEGlobalsConverter &) = delete;
   LLVM2ICEGlobalsConverter &
   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))
       return CI->getSExtValue();
 
     std::string Buffer;
     raw_string_ostream StrBuf(Buffer);
     StrBuf << "Constant not i32 literal: " << *C;
     report_fatal_error(StrBuf.str());
     return 0;
   }
 };
 
 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) {
     if (!I->hasInitializer() && Ctx->getFlags().AllowUninitializedGlobals)
       continue;
 
     const auto GV = dyn_cast<GlobalVariable>(I);

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

cast<> is a checked cast and would help assert(GV); ?

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

Ok. Changing to cast and removing assert.

     assert(GV);
     Ice::IceString Name = GV->getName();
     if (!GV->hasInternalLinkage()) {
       std::string Buffer;
       raw_string_ostream StrBuf(Buffer);
       StrBuf << "Can't define external global address: " << Name;
       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)) {
       for (ConstantStruct::const_op_iterator I = CompoundInit->op_begin(),
                                              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;
   }
 
   if (isa<ConstantAggregateZero>(Initializer)) {
     if (const auto AT = dyn_cast<ArrayType>(Initializer->getType())) {
       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");
     }
     return;
   }
 
   if (const auto Exp = dyn_cast<ConstantExpr>(Initializer)) {
     switch (Exp->getOpcode()) {
     case Instruction::Add:
       assert(!HasOffset);
       addGlobalInitializer(Global, Exp->getOperand(0), true,
                            getIntegerLiteralConstant(Exp->getOperand(1)));
       return;
     case Instruction::PtrToInt: {
       assert(TypeConverter.convertToIceType(Exp->getType()) ==
              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;
     }
   }
 
   std::string Buffer;
   raw_string_ostream StrBuf(Buffer);
   StrBuf << "Unhandled global initializer: " << Initializer;
   report_fatal_error(StrBuf.str());
 }
 
 } // end of anonymous namespace
 
 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;
 
     TimerIdT TimerID = 0;
     if (Ctx->getFlags().TimeEachFunction) {
       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);
     if (Ctx->getFlags().TimeEachFunction)
       Ctx->popTimer(TimerID, StackID);
   }
 
   emitConstants();
 }
 
 } // end of namespace Ice