[PATCH] D153839: [XCOFF] Force recording a relocation for weak symbol label.

[ChenZheng via Phabricator via llvm-commits]

shchenz accepted this revision as: shchenz.
shchenz added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for addressing this bug.



================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:736
+    FixedValue = getVirtualAddress(SymA, SymASec) - BRInstrAddress +
+                 Target.getConstant();
   } else if (Type == XCOFF::RelocationType::R_REF) {
----------------
Hmm, ok, seems before this change, we assume `R_RBR` type relocation is only applied to function sections, so that we only need the virtual address of the section because the target function offset in the section are always 0.

Now, we will generate `R_RBR` for functions that has no own csect, so using section virtual address is now not enough, we also need to consider about the offset of the function in the "big" csect.

This change makes sense to me.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp:182
             return true;
-        }
+        } else if (const auto *S = dyn_cast<MCSymbolXCOFF>(&A->getSymbol())) {
+          return !Target.isAbsolute() && S->isExternal() &&
----------------
shchenz wrote:
> This seems not equal to what we handle for Linux(where we have local entry). On Linux, for non weak functions, they also need a force relocation for usage like compiling a shared library. I've asked help internally.
Like what we discussed internally, I think this is enough for AIX.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153839