[lld] r298672 - Early continue. NFC.

[Sean Silva via llvm-commits]

If you're refactoring this code, one request I have is to look at the calls
to isPreemptible. We call it multiple times (sometimes in subroutines).
IIRC when I was just staring at the code (for some unrelated reason) it
wasn't obvious at a glance whether all the different calls compute the same
value. Saving isPreemptible in a variable where possible would make this
clearer (and when the value does need to be recomputed, then some comments
would be really helpful explaining why).

For example, adjustExpr can modify NeedsPltAddr, and isPreemptible depends
on that.

-- Sean Silva

On Mar 23, 2017 7:02 PM, "Rui Ueyama via llvm-commits" <
llvm-commits at lists.llvm.org> wrote:

Author: ruiu
Date: Thu Mar 23 19:50:37 2017
New Revision: 298672

URL: http://llvm.org/viewvc/llvm-project?rev=298672&view=rev
Log:
Early continue. NFC.

The original code is a big `if` and `else` which ends with `continue`
like this:

  if (cond) {
    ...
    // fall through
  } else {
    ...
    continue;
  }

This patch rewrites it with the following.

  if (!cond) {
    ...
    continue;
  }
  ...

Modified:
    lld/trunk/ELF/Relocations.cpp

Modified: lld/trunk/ELF/Relocations.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/
Relocations.cpp?rev=298672&r1=298671&r2=298672&view=diff
============================================================
==================
--- lld/trunk/ELF/Relocations.cpp (original)
+++ lld/trunk/ELF/Relocations.cpp Thu Mar 23 19:50:37 2017
@@ -736,26 +736,8 @@ static void scanRelocs(InputSectionBase
     if (Expr == R_TLSDESC_CALL)
       continue;

-    if (needsPlt(Expr) ||
-        refersToGotEntry(Expr) || !isPreemptible(Body, Type)) {
-      // If the relocation points to something in the file, we can process
it.
-      bool Constant =
-          isStaticLinkTimeConstant<ELFT>(Expr, Type, Body, C, RI.r_offset);
-
-      // If the output being produced is position independent, the final
value
-      // is still not known. In that case we still need some help from the
-      // dynamic linker. We can however do better than just copying the
incoming
-      // relocation. We can process some of it and and just ask the dynamic
-      // linker to add the load address.
-      if (!Constant)
-        AddDyn({Target->RelativeRel, &C, Offset, true, &Body, Addend});
-
-      // If the produced value is a constant, we just remember to write it
-      // when outputting this section. We also have to do it if the format
-      // uses Elf_Rel, since in that case the written value is the addend.
-      if (Constant || !RelTy::IsRela)
-        C.Relocations.push_back({Expr, Type, Offset, Addend, &Body});
-    } else {
+    if (!needsPlt(Expr) && !refersToGotEntry(Expr) &&
+        isPreemptible(Body, Type)) {
       // We don't know anything about the finaly symbol. Just ask the
dynamic
       // linker to handle the relocation for us.
       if (!Target->isPicRel(Type))
@@ -783,6 +765,24 @@ static void scanRelocs(InputSectionBase
       continue;
     }

+    // If the relocation points to something in the file, we can process
it.
+    bool Constant =
+        isStaticLinkTimeConstant<ELFT>(Expr, Type, Body, C, RI.r_offset);
+
+    // If the output being produced is position independent, the final
value
+    // is still not known. In that case we still need some help from the
+    // dynamic linker. We can however do better than just copying the
incoming
+    // relocation. We can process some of it and and just ask the dynamic
+    // linker to add the load address.
+    if (!Constant)
+      AddDyn({Target->RelativeRel, &C, Offset, true, &Body, Addend});
+
+    // If the produced value is a constant, we just remember to write it
+    // when outputting this section. We also have to do it if the format
+    // uses Elf_Rel, since in that case the written value is the addend.
+    if (Constant || !RelTy::IsRela)
+      C.Relocations.push_back({Expr, Type, Offset, Addend, &Body});
+
     // At this point we are done with the relocated position. Some
relocations
     // also require us to create a got or plt entry.



_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170324/125c2397/attachment.html>