[PATCH] D73542: [LLD][ELF][ARM] Do not substitute BL/BLX for non STT_FUNC symbols.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lld/ELF/Arch/ARM.cpp:412
+    bool isBlx = (read32le(loc) & 0xfe000000) == 0xfa000000;
+    bool interwork = (rel.sym && rel.sym->isFunc()) && rel.type != R_PLT_PC;
+    if ((interwork && (val & 1)) || (!interwork && isBlx)) {
----------------
excess pair of `()`

(The `!rel.sym` case is not covered by a test, but I think it does not matter.)


================
Comment at: lld/ELF/Arch/ARM.cpp:413
+    bool interwork = (rel.sym && rel.sym->isFunc()) && rel.type != R_PLT_PC;
+    if ((interwork && (val & 1)) || (!interwork && isBlx)) {
       // The BLX encoding is 0xfa:H:imm24 where Val = imm24:H:'1'
----------------
One simplification is `interwork ? val & 1 : isBlx`.


================
Comment at: lld/ELF/Arch/ARM.cpp:420
       break;
-    }
-    if ((read32le(loc) & 0xfe000000) == 0xfa000000)
+    } else {
       // BLX (always unconditional) instruction to an ARM Target, select an
----------------
`else` can be dropped.


================
Comment at: lld/ELF/Arch/ARM.cpp:458
+    bool isBlx = (read16le(loc + 2) & 0x1000) == 0;
+    bool interwork = (rel.sym && rel.sym->isFunc()) || rel.type == R_PLT_PC;
+    if ((interwork && (val & 1) == 0) || (!interwork && isBlx)) {
----------------
(The !rel.sym case is not covered by a test, but I think it does not matter.)


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

https://reviews.llvm.org/D73542