[PATCH] D60353: ELF: Add basic partition data structures and behaviours.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In D60353#1506498 <https://reviews.llvm.org/D60353#1506498>, @ruiu wrote:

> Do you have another change for adding synthetic sections for partitioning?


Yes, I was going to send it out once it was ready, but I'll try to send it out as soon as I can so you can see what it looks like. In the meantime you could look at https://github.com/pcc/llvm-project/tree/lld-part-symbols which is essentially the same code as I'm going to upstream.

> This change looks pretty straightforward to me, although I'd like to somehow continue using the term "Live" instead of Partition is zero" to explain liveness in code and comments, though.

I added a couple of accessors `markLive` and `markDead` for the places where we were setting Partition directly before.



================
Comment at: lld/ELF/Driver.cpp:1378
+static void readSymbolPartitionSection(InputSectionBase *S) {
+  Symbol *Sym;
+  if (S->AreRelocsRela)
----------------
ruiu wrote:
> Could you add a brief comment as to what this Sym is? I think this is a "root" symbol for each partition.
Yes, pretty much. I've added a comment saying that this is the entry point symbol. Following the link above will lead to an explanation (once D60242 lands) of what an "entry point symbol" is.


================
Comment at: lld/ELF/Driver.cpp:1400
+          ": partitions cannot be used with the SECTIONS command");
+  if (Script->hasPhdrsCommands())
+    error(toString(S->File) +
----------------
ruiu wrote:
> I first thought that we may want to completely ban linker scripts when this feature is in use, but no, we can't do that, because on Linux libc.so is actually a linker script...
Right.


================
Comment at: lld/ELF/InputSection.h:78
 
+  unsigned : 1;
+
----------------
ruiu wrote:
> It's not clear to me why you needed a one bit padding here.
I added a comment explaining why.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60353/new/

https://reviews.llvm.org/D60353