This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfafa48e7b518: [AIX][clang][driver] Check the command string
to the linker for exportlist opts (authored by zhijian
zhij...@ca.ibm.com).
daltenty accepted this revision.
daltenty added a comment.
LGTM, thanks
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119147/new/
https://reviews.llvm.org/D119147
___
cfe-commits mailing list
DiggerLin updated this revision to Diff 455758.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119147/new/
https://reviews.llvm.org/D119147
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Driver/Job.h
clang/lib/Driver/Job.cpp
daltenty added a comment.
Suggest adding the following text to `clang/docs/ReleaseNotes.rst` under the
AIX section with this change:
* When using `-shared`, the clang driver now invokes llvm-nm to create an
export list if the user doesn't specify one via linker flag or pass an
alternative
DiggerLin added a comment.
if there is no more comment. Can you help to approve it ?, thanks in advance.
@MaskRay @daltenty
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119147/new/
https://reviews.llvm.org/D119147
DiggerLin updated this revision to Diff 427297.
DiggerLin marked an inline comment as done.
DiggerLin added a comment.
address MaskRay's comment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119147/new/
https://reviews.llvm.org/D119147
Files:
stevewan added inline comments.
Comment at: clang/test/Driver/aix-ld.c:675-676
+// CHECK-LD32-SHARED-EXPORTS-NOT: "{{.*}}llvm-nm"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "-X"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "32"
+// CHECK-LD32-SHARED-EXPORTS: "{{.*}}ld{{(.exe)?}}"
DiggerLin marked 8 inline comments as done.
DiggerLin added inline comments.
Comment at: clang/test/Driver/aix-ld.c:985
+// CHECK-LD64-SHARED-EXPFULL: "-bM:SRE"
+// CHECK-LD64-SHARED-EXPFULL: "-bnoentry"
+// CHECK-LD64-SHARED-EXPFULL: "-b64"
MaskRay
DiggerLin marked an inline comment as done.
DiggerLin added inline comments.
Comment at: clang/test/Driver/aix-ld.c:675-676
+// CHECK-LD32-SHARED-EXPORTS-NOT: "{{.*}}llvm-nm"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "-X"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "32"
+//
MaskRay added inline comments.
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:196
+const char *CreateExportListExec = Args.MakeArgString(
+llvm::sys::path::parent_path(ToolChain.getDriver().ClangExecutable) +
+"/llvm-nm");
Use
stevewan accepted this revision.
stevewan added a comment.
This revision is now accepted and ready to land.
LGTM other than some nits.
Comment at: clang/lib/Driver/Job.cpp:361
+
+ if (!RedirectFiles.empty()) {
+std::vector> RedirectFilesOptional;
DiggerLin added inline comments.
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:80
+static bool hasExportListLinkerOpts(const ArgStringList ) {
+ for (size_t i = 0, Size = CmdArgs.size(); i < Size; ++i) {
+llvm::StringRef ArgString(CmdArgs[i]);
stevewan
DiggerLin updated this revision to Diff 423972.
DiggerLin marked 2 inline comments as done.
DiggerLin added a comment.
address comment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119147/new/
https://reviews.llvm.org/D119147
Files:
stevewan added a comment.
I don't think the redirect files portion of this patch is well-described in the
description, could you please add it?
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:80
+static bool hasExportListLinkerOpts(const ArgStringList ) {
+ for (size_t i =
DiggerLin added inline comments.
Comment at: clang/test/Driver/aix-ld.c:609
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared (with exp option strings in
other opt).
+// RUN: %clangxx -x c++ -no-canonical-prefixes %s 2>&1 -### \
+// RUN:
DiggerLin updated this revision to Diff 412516.
DiggerLin marked an inline comment as done.
Herald added a project: All.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119147/new/
https://reviews.llvm.org/D119147
Files:
DiggerLin updated this revision to Diff 411172.
DiggerLin marked 3 inline comments as done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119147/new/
https://reviews.llvm.org/D119147
Files:
clang/include/clang/Driver/Job.h
MaskRay added inline comments.
Comment at: clang/test/Driver/aix-ld.c:609
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared (with exp option strings in
other opt).
+// RUN: %clangxx -x c++ -no-canonical-prefixes %s 2>&1 -### \
+// RUN:
DiggerLin marked 6 inline comments as done.
DiggerLin added inline comments.
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:80
+static bool hasExportListLinkerOpts(const ArgStringList ) {
+ for (size_t i = 0; i < CmdArgs.size(); ++i) {
+llvm::StringRef
MaskRay added inline comments.
Comment at: clang/lib/Driver/Job.cpp:304
+void Command::setRedirectFiles(std::vector> Redirects) {
+ RedirectFiles = Redirects;
Comment at: clang/lib/Driver/Job.cpp:370
+
DiggerLin updated this revision to Diff 406560.
DiggerLin added a comment.
run git format
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119147/new/
https://reviews.llvm.org/D119147
Files:
clang/include/clang/Driver/Job.h
DiggerLin created this revision.
DiggerLin added reviewers: daltenty, hubert.reinterpretcast.
DiggerLin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
We currently only check driver Wl options and don't check for the plain
-b, -Xlinker or
22 matches
Mail list logo
