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

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 29 12:33:48 PDT 2020


hubert.reinterpretcast added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-llvm-intrinsic.ll:42
+; COMMON-NEXT:  .csect .text[PR]
+; COMMON-NEXT: .bar:
+; COMMON-NEXT: # %bb.0:                                # %entry
----------------
Start with `COMMON-LABEL` with this line and omit the lines above. The existence of the function descriptor is not the purpose of this test. After this, there would be no difference between 32-bit and 64-bit and we should be able to replace the prefix with plain `CHECK`.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-llvm-intrinsic.ll:45
+; COMMON-NEXT:  mflr 0
+; COMMON:       bl .memset
+
----------------
I think the object code generation path test needs to verify that the relocation refers to the correct symbol table entry.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-llvm-intrinsic.ll:51
+; COMMON-NEXT:  .tc s[TC],s[UA]
+; COMMON-NEXT:  .extern .memset
+
----------------
I think we should similarly omit the lines in this block before this last one.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-llvm-intrinsic.ll:74
+; CHECKSYM-NEXT:   }
+; CHECKSYM-NEXT:   Symbol {
+; CHECKSYM-NEXT:     Index: [[#Index+2]]
----------------
I think we can omit the symbol table entries aside from the one for `.memset`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78929





More information about the llvm-commits mailing list