[PATCH] D27398: Use "equivalence class" instead of "color" to describe the concept in ICF.

Sean Silva via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Dec 4 19:01:58 PST 2016


silvas added a comment.

LGTM with a couple wording suggestions. Thanks.

Btw, did we lose the citation to the Gold ICF paper? I think we probably ought to mention it since the approach was inspired from there IIRC.



================
Comment at: ELF/ICF.cpp:38
+//    values contain section types, section contents and numbers of
+//    relocations. At this moment, relocation targets are not taken into
+//    account. We just put sections that apparently differ into different
----------------
Slight clarify improvement: s/At this moment/During this step/

Otherwise, "at this moment" might be interpreted as "this feature is not implemented" (though this doesn't make sense in context, but to a casual reader that would not necessarily be obvious)


================
Comment at: ELF/ICF.cpp:43
+// 2. Next, for each equivalence class, we visit sections to compare
+//    relocation targets. Relocation targets are compared by their
+//    equivalence class. Sections with different relocation targets are
----------------
Small wording suggestion:

I would suggest replacing `Relocation targets are compared by their equivalence class` with `Relocation targets are considered equivalent if their targets are in the same equivalence class`. That is a bit more concrete.



https://reviews.llvm.org/D27398





More information about the llvm-commits mailing list