[PATCH] D78929: [AIX][XCOFF]emit extern linkage for the llvm intrinsic symbol

[Digger via Phabricator via llvm-commits]

DiggerLin marked 6 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-llvm-intrinsic.ll:9-11
+; RUN: llvm-readobj --symbols %t.o | FileCheck --check-prefix=CHECKSYM %s
+; RUN: llvm-readobj --relocations --expand-relocs %t.o | FileCheck --check-prefix=CHECKRELOC %s
+
----------------
jhenderson wrote:
> jasonliu wrote:
> > hubert.reinterpretcast wrote:
> > > DiggerLin wrote:
> > > > DiggerLin wrote:
> > > > > jhenderson wrote:
> > > > > > DiggerLin wrote:
> > > > > > > jhenderson wrote:
> > > > > > > > You can fold these two lines into one. No need for them to be separate:
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > ; RUN: llvm-readobj --symbols --relocations --expand-relocs %t.o | \
> > > > > > > > ; RUN:   FileCheck --check-prefix=SYMRELOC %s
> > > > > > > > ```
> > > > > > > Thanks for your suggestion, but I think separating in two is based is more reasonable ,One test for test Symbol. other test for test relocation.  it is more readable.
> > > > > > I'm not sure how it's more readable. It's very clear from my suggested command-line that we want to test the symbols and relocations (otherwise the options wouldn't be present), and would reduce the amount of work required to figure out which checks below correspond to which FileCheck line.
> > > > > > 
> > > > > > It would also be more in keeping with most tests I've worked with, and would reduce the number of executable invocations from 4 to 2, improving test run time.
> > > > > I tried it with llvm-readobj --symbols --relocations --expand-relocs
> > > > > it display the relocation information first and than  symbol information.
> > > > > 
> > > > > in the test case we want to verify symbol first and than relocation.
> > > > > I have to keep current test.
> > > > @hubert.reinterpretcast , what is your option on combine the
> > > > ```  llvm-readobj --symbols ```
> > > > and 
> > > >  ```llvm-readobj --relocations --expand-relocs```
> > > >  into 
> > > > ```llvm-readobj --symbols --relocations --expand-relocs```
> > > > 
> > > > and than verify relocation information first as
> > > > 
> > > > ; SYMRELOC:    Relocation {{[{][[:space:]] *}}Virtual Address: 0x18
> > > > ; SYMRELOC-NEXT:    Symbol: .memset (0)
> > > > ; SYMRELOC-NEXT:    IsSigned: Yes
> > > > ; SYMRELOC-NEXT:    FixupBitValue: 0
> > > > ; SYMRELOC-NEXT:    Length: 26
> > > > ; SYMRELOC-NEXT:    Type: R_RBR (0x1A)
> > > > ; SYMRELOC-NEXT:  }
> > > > 
> > > > ; SYMRELOC:      Symbol {
> > > > ; SYMRELOC-NEXT:     Index: 0
> > > > ; SYMRELOC-NEXT:     Name: .memset
> > > > ; SYMRELOC-NEXT:     Value (RelocatableAddress): 0x0
> > > > ; SYMRELOC-NEXT:     Section: N_UNDEF
> > > > ; SYMRELOC-NEXT:     Type: 0x0
> > > > ; SYMRELOC-NEXT:     StorageCl
> > > > ......
> > > I suggest using `llvm-objdump -d --symbol-description` for the relocation check. As it is, we can't tell whether the relocation we found is in the right place. This happens to make it so that we end up with two separate tool invocations.
> > I think you meant: `llvm-objdump -d -r --symbol-description`
> It's moot, given the `llvm-objdump` suggestion, but I'm not sure it really matters which order symbols or relocations are printed in.
because there is symbol index in the relocation information. we prefer to  verify the symbol first and then relocation.  otherwise, when we verified relocation, we have to  go down the test file to search the symbol information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78929