[PATCH] D22674: [ELF] - Linkerscript: implemented ALIGN modificatior of output sections.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 26 05:58:17 PDT 2016


grimar added a comment.

In https://reviews.llvm.org/D22674#495326, @ruiu wrote:

> Instead of adding new code to the function, can you create a new function that processes only ALIGN option?


I moved the handling to a assignAddresses() method, IMO more appropriate for that, and so handling is just 2 lines now. 
I am not sure if moving to new function worth that now, when it is just 2 lines ? I guess no.
And the reason I moved it to assignAddresses() is that I found that location counter can be used in
ALIGN, like:
.aaa : ALIGN(4096 + . * 100) { ... }  
Though I never saw this in real linkerscripts, it still looks as a more correct place, since we know the value
of location counter only in that place. What do you think ?


================
Comment at: ELF/LinkerScript.cpp:664-665
@@ -652,1 +663,4 @@
 
+  if (peek() == "ALIGN")
+    readAlign(Cmd);
+
----------------
ruiu wrote:
>   if (skip("ALIGN"))
>     readAlign(Cmd);
Done.


https://reviews.llvm.org/D22674





More information about the llvm-commits mailing list