[PATCH] [lld] [LinkerScript] Implement linker script expression evaluation

Shankar Kalpathi Easwaran shankarke at gmail.com
Sun Mar 8 19:57:25 PDT 2015


================
Comment at: include/lld/ReaderWriter/ELFLinkingContext.h:304-306
@@ -303,1 +303,5 @@
 
+  const std::vector<std::unique_ptr<script::Parser>> &scripts() const {
+    return _scripts;
+  }
+
----------------
do you want to add a boolean hasLinkerScript that checks the size and returns ?

================
Comment at: include/lld/ReaderWriter/LinkerScript.h:366
@@ +365,3 @@
+  // here is an overkill.
+  typedef llvm::StringMap<int64_t, llvm::BumpPtrAllocator> SymbolTableTy;
+
----------------
you could replace this with a llvm::DenseMap<StringRef, int64_t> and use a common StringRefMappingInfo.

================
Comment at: include/lld/ReaderWriter/LinkerScript.h:794-795
@@ -767,2 +793,4 @@
   void dump(raw_ostream &os) const override;
+  const_iterator begin() const { return _sectionsCommands.begin(); }
+  const_iterator end() const { return _sectionsCommands.end(); }
 
----------------
Use lld::range,once you replace it with range, you can remove the typedef for const_iterator.

================
Comment at: lib/ReaderWriter/LinkerScript.cpp:581
@@ +580,3 @@
+  switch (_op) {
+  case Unary::Minus:
+    return 0 - childRes;
----------------
Do you want to handle Unary::Plus ?? 

================
Comment at: lib/ReaderWriter/LinkerScript.cpp:584
@@ +583,3 @@
+  case Unary::Not:
+    return ~childRes;
+  }
----------------
I thought linker script uses only a boolean value of a Unary Not, no ?? 

================
Comment at: lib/ReaderWriter/LinkerScript.cpp:703
@@ +702,3 @@
+    return trueExprRes;
+  else
+    return falseExprRes;
----------------
no need of else.

================
Comment at: unittests/DriverTests/GnuLdDriverTest.cpp:262
@@ +261,3 @@
+  EXPECT_EQ(script::SymbolAssignment::Simple, sa1->assignmentKind());
+  EXPECT_EQ(script::SymbolAssignment::Normal, sa1->assignmentVisibility());
+
----------------
Can we change SymbolAssignment::Normal to SymbolAssignment::Default ? 

Looks like you are using the same SymboAssignment to name two different things, assignmentKind and assignmentVisibilty. I would recommend separating them.

http://reviews.llvm.org/D8156

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






More information about the llvm-commits mailing list