[PATCH] D82816: [LLD][PowerPC] Implement R_PPC64_REL24_NOTOC calls, callee also has no TOC

Thu Jul 9 10:36:54 PDT 2020

sfertile accepted this revision.
sfertile added a comment.

One comment but otherwise LGTM.

================
Comment at: lld/ELF/Arch/PPC64.cpp:1041
     return false;

   // If a function is in the Plt it needs to be called with a call-stub.
----------------
NeHuang wrote:
> sfertile wrote:
> > We should probably insert a couple of fatal error here:
> > 1) if the type in NOTOC and the symbols st_other indicates it needs the toc-pointer setup.
> > 2) If the type is not NOTOC but the symbols st_other indicates it tramples the toc.
> Thanks Sean for the advice. I also moved the fatal error check for the protocol "external call with R_PPC64_REL_NOTOC" here so that we are checking all unimplemented protocols in the same function. 
👍 

================
Comment at: lld/test/ELF/ppc64-pcrel-call-to-pcrel.s:13
+# RUN: ld.lld -T %t.script -shared %t1.o %t2.o -o %t.so
+# RUN: ld.lld -T %t.script %t3.o -o %t
+# RUN: llvm-readelf -s %t.so | FileCheck %s --check-prefix=SYMBOL
----------------
Nit: I suggest having a separate set of run steps for testing the exec since the steps are completely disjoint from the shared object test. ie separate out
```
# RUN: llvm-mc -filetype=obj -triple=powerpc64[le] -defsym GLOBAL=1 %s -o %t3.o
# RUN: ld.lld -T %t.script %t3.o -o %t
# RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=SYMBOL-GLOBAL
# RUN: llvm-objdump -d --no-show-raw-insn --mcpu=pwr10 %t | FileCheck %s
```

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82816