[PATCH] add support for LayoutAfter/LayoutBefore references

Shankar Kalpathi Easwaran shankarke at gmail.com
Wed Feb 6 18:34:32 PST 2013


  The current implementation would just not layout A before B because C is followed by B. I think layout-before and layout-after gives more flexibility to the user when it comes to ordering atoms, any invalid layouts specified by the user is just ignored by the implementation with an appropriate warning.


================
Comment at: include/lld/Core/Reference.h:55-59
@@ +54,7 @@
+    case -1:
+      return "layoutbefore";
+      break;
+    case -2:
+      return "layoutafter";
+      break;
+    default:
----------------
kledzik at apple.com wrote:
> The usual YAML style is a dash between words (e.g. layout-before).  Where else are these string used?
This and TestingHelpers

================
Comment at: lib/ReaderWriter/ELF/Atoms.h:78-82
@@ -80,7 +77,7 @@
 private:
-  const Atom  *_target;
-  uint64_t     _targetSymbolIndex;
-  uint64_t     _offsetInAtom;
-  Addend       _addend;
-  Kind         _kind;
+  const Atom *_target;
+  uint64_t _targetSymbolIndex;
+  uint64_t _offsetInAtom;
+  Addend _addend;
+  Kind _kind;
 };
----------------
kledzik at apple.com wrote:
> :-(.  I find having the ivar names vertically aligned to be much more readable.  Why did you go out of your way to remove that?
This is done by clang-format. I ran the diff by clang-format and that did the rest :)

================
Comment at: include/lld/Core/Reference.h:38-39
@@ +37,4 @@
+  enum {
+    LayoutBefore = -1,
+    LayoutAfter = -2
+  };
----------------
kledzik at apple.com wrote:
> The type should be encoded in these names.  These values are "kinds", so better names would be kindLayoutBefore and kindLayoutAfter.
Sure.

================
Comment at: test/followon-test.objtxt:30
@@ +29,2 @@
+# CHKORDER:   - name:            fn2
+# CHKORDER:   - name:            fn3
----------------
kledzik at apple.com wrote:
> Since the kind name is "layout after" seems like the test file name should match (layout-after.objtxt).
Ok.


http://llvm-reviews.chandlerc.com/D373



More information about the llvm-commits mailing list