qiongsiwu1 added a comment.
https://reviews.llvm.org/D150586 has landed and the problematic test case is
removed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
_
qiongsiwu1 added a comment.
Patch to remove the problematic test case. https://reviews.llvm.org/D150586
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
___
cfe-
qiongsiwu1 added a comment.
This patch is causing some buildbot failures. Specifically
https://lab.llvm.org/buildbot/#/builders/139/builds/40652
https://lab.llvm.org/buildbot/#/builders/258/builds/810
I am looking into them. Please revert if this cannot wait. Thanks!
Repository:
rG LLVM Gith
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9715af434579: [AIX][clang] Storage Locations for Constant
Pointers (authored by qiongsiwu1).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LA
qiongsiwu1 added a comment.
Pre-commit CI x64 Debian is failing
https://buildkite.com/llvm-project/premerge-checks/builds/152189 because the
build machine's cmake version is too old. I am landing this patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llv
qiongsiwu1 updated this revision to Diff 522132.
qiongsiwu1 added a comment.
Thanks so much @hubert.reinterpretcast !! The comments are addressed. I will
land the patch when the pre-commit CI finishes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.
LGTM with minor comments!
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:132
+// possible. Then `-bforceimprw` changes such sections to R
qiongsiwu1 updated this revision to Diff 520375.
qiongsiwu1 added a comment.
Rebase. Ping for review @hubert.reinterpretcast . Thanks so much!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Files:
clan
qiongsiwu1 added a comment.
@hubert.reinterpretcast please let me know if there are comments/suggestions!
Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
_
qiongsiwu1 added a comment.
Ping for review. Thank you!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
qiongsiwu1 added a comment.
Ping for review. Specifically, the option is renamed to `-mxcoff-roptr` and
`-mno-xcoff-roptr` to indicate that this option only affects the `XCOFF` binary
format, so it is expected to not affect `ELF` or other formats.
Thanks!
Repository:
rG LLVM Github Monorepo
qiongsiwu1 added inline comments.
Comment at: clang/test/Driver/ppc-roptr.c:37
+// LINK: "-bforceimprw"
+// LTO_ROPTR: "-bplugin_opt:-mxcoff-roptr"
+// NO_ROPTR-NOT: "-mxcoff-roptr"
qiongsiwu1 wrote:
> hubert.reinterpretcast wrote:
> > This needs the backend opti
qiongsiwu1 marked an inline comment as done.
qiongsiwu1 added inline comments.
Comment at: clang/test/Driver/ppc-roptr.c:37
+// LINK: "-bforceimprw"
+// LTO_ROPTR: "-bplugin_opt:-mxcoff-roptr"
+// NO_ROPTR-NOT: "-mxcoff-roptr"
hubert.reinterpretcast wrote:
> This
qiongsiwu1 updated this revision to Diff 509367.
qiongsiwu1 marked an inline comment as done.
qiongsiwu1 added a comment.
Fix the LTO data sections check.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
F
qiongsiwu1 updated this revision to Diff 509360.
qiongsiwu1 added a comment.
Moving the `-shared` check to the AIX linker and modifying the tests so we test
pure linking.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.ll
qiongsiwu1 updated this revision to Diff 509342.
qiongsiwu1 added a comment.
Rebase and address feedback other than test cases.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Files:
clang/docs/ReleaseN
hubert.reinterpretcast added inline comments.
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:125
+ // The `-mroptr` option places constants in RO sections as much as possible.
+ // Then `-bforceimprw` changes such sections to RW if they contain imported
Old
hubert.reinterpretcast added inline comments.
Comment at: clang/docs/ReleaseNotes.rst:317
+ ``-fno-data-sections``. When ``-mxcoff-roptr`` is in effect at link time,
+ read-only data sections with relocatable address values that resolve to
+ imported symbols are made writable
hubert.reinterpretcast added inline comments.
Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:647
+def err_roptr_requires_data_sections: Error<"-mxcoff-roptr is supported only
with -fdata-sections">;
+def err_roptr_cannot_build_shared: Error<"-mxcoff-roptr is not
qiongsiwu1 added a comment.
The new option is renamed to `-mxcoff-roptr` to avoid confusions if we are
using it during a pure linking job on non-AIX platforms. The option name now
implies that it applies to XCOFF only, so silently ignoring the option on other
platforms is not that serious a pro
qiongsiwu1 updated this revision to Diff 509118.
qiongsiwu1 edited the summary of this revision.
qiongsiwu1 added a comment.
Renaming the `-mroptr` option to `-mxcoff-roptr`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews
qiongsiwu1 updated this revision to Diff 509091.
qiongsiwu1 added a comment.
Simplify LTO data section error check.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Files:
clang/docs/ReleaseNotes.rst
c
qiongsiwu1 updated this revision to Diff 509057.
qiongsiwu1 added a comment.
Fixing LTO data sections error check.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Files:
clang/docs/ReleaseNotes.rst
cl
qiongsiwu1 added inline comments.
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:714-716
+if (!Args.hasFlag(options::OPT_fdata_sections,
+ options::OPT_fno_data_sections, UseSeparateSections) &&
+Args.hasArg(options::OPT_fno_data_sections)
hubert.reinterpretcast added inline comments.
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:714-716
+if (!Args.hasFlag(options::OPT_fdata_sections,
+ options::OPT_fno_data_sections, UseSeparateSections) &&
+Args.hasArg(options::OPT_fno_da
qiongsiwu1 updated this revision to Diff 507752.
qiongsiwu1 added a comment.
Address code review. Thanks for your comments/suggestions @MaskRay !!
- Fixing comments.
- Using `--target=` in tests.
- Reducing number of RUN lines in tests.
@hubert.reinterpretcast I ended up having a mix of tests fo
qiongsiwu1 added inline comments.
Comment at: clang/test/Driver/ppc-roptr.c:21
+// RUN: FileCheck %s --check-prefix=ROPTR
+// RUN: %clang -### -target powerpc64-ibm-aix-xcoff -S -mroptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=ROPTR
MaskRay wrote:
hubert.reinterpretcast added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5250
+ if (Args.hasArg(options::OPT_mroptr) || Args.hasArg(options::OPT_mno_roptr))
{
+bool HasRoptr =
qiongsiwu1 wrote:
> hubert.reinterpretcast wrote:
> > Th
MaskRay added inline comments.
Comment at: clang/test/Driver/ppc-roptr.c:1
+// RUN: %clang -### -target powerpc-ibm-aix-xcoff -mroptr %s 2>&1 | \
+// RUN: FileCheck %s --check-prefixes=ROPTR,LINK
Prefer `--target=` to `-target ` for new tests.
=
MaskRay added inline comments.
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:125
+ // `-mroptr` implies the `-bforceimprw` linker option.
+ // The `-mroptr` option places constants in RO sections as much as possible.
Delete `// `-mroptr` implies the `-bforc
qiongsiwu1 added a comment.
Ping for review/comments. Thanks so much!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
___
cfe-commits mailing list
cfe-commits@l
qiongsiwu1 added a comment.
Ping for review/comments. Thanks so much!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
___
cfe-commits mailing list
cfe-commits@l
qiongsiwu1 added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5250
+ if (Args.hasArg(options::OPT_mroptr) || Args.hasArg(options::OPT_mno_roptr))
{
+bool HasRoptr =
hubert.reinterpretcast wrote:
> This only checks for `-m[no-]roptr`
qiongsiwu1 updated this revision to Diff 505181.
qiongsiwu1 added a comment.
Thanks for the comments @stephenpeckham ! This update addresses all of them.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Fi
stephenpeckham added inline comments.
Comment at: clang/docs/ReleaseNotes.rst:230
+- Introduced the ``-mroptr`` option to place constant objects with relocatable
+ address values in the read-only data section. This option is intended to
+ be used with the ``-fdata-sections`` op
qiongsiwu1 added a comment.
Ping for review. Thank you!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
qiongsiwu1 added a comment.
Ping for review. Thank you!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
qiongsiwu1 updated this revision to Diff 503946.
qiongsiwu1 added a comment.
Adding LTO error check tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Files:
clang/docs/ReleaseNotes.rst
clang/incl
qiongsiwu1 updated this revision to Diff 503944.
qiongsiwu1 added a comment.
Adding tests to check that the default behaviour is identical with `-mno-roptr`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
qiongsiwu1 updated this revision to Diff 503941.
qiongsiwu1 added a comment.
Fixing release note. Adding pure linking tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Files:
clang/docs/ReleaseNote
qiongsiwu1 updated this revision to Diff 503934.
qiongsiwu1 added a comment.
Adding logic to pass `mroptr` to the backend during LTO codegen. Error check is
not ideal.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.
qiongsiwu1 updated this revision to Diff 503796.
qiongsiwu1 added a comment.
Rename the `ReadOnlyPointers` option in the source to `XCOFFReadOnlyPointers`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
hubert.reinterpretcast added inline comments.
Comment at: clang/test/Driver/ppc-roptr.c:10
+// ROPTR-NOT: "-mroptr"
+// ROPTR-NOT: "-bforceimprw"
+
hubert.reinterpretcast wrote:
> w2yehia wrote:
> > hubert.reinterpretcast wrote:
> > > Do we pass the option throug
hubert.reinterpretcast added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5250
+ if (Args.hasArg(options::OPT_mroptr) || Args.hasArg(options::OPT_mno_roptr))
{
+bool HasRoptr =
This only checks for `-m[no-]roptr` when the front-end i
hubert.reinterpretcast added inline comments.
Comment at: clang/docs/ReleaseNotes.rst:231
+ address values in the read-only data section. This option is intended to
+ be used with the ``-fdata-sections`` option. When ``-mroptr`` is in effect,
+ read-only data sections with rel
hubert.reinterpretcast added inline comments.
Comment at: clang/test/Driver/ppc-roptr.c:10
+// ROPTR-NOT: "-mroptr"
+// ROPTR-NOT: "-bforceimprw"
+
w2yehia wrote:
> hubert.reinterpretcast wrote:
> > Do we pass the option through to the LTO codegen when `-fprofile
qiongsiwu1 updated this revision to Diff 503440.
qiongsiwu1 added a comment.
Update `-mroptr` cc1 option help text.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Files:
clang/docs/ReleaseNotes.rst
c
qiongsiwu1 updated this revision to Diff 503437.
qiongsiwu1 added a comment.
Addressing review comments - fixing typos, and updating the tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Files:
cla
qiongsiwu1 added inline comments.
Comment at: clang/test/Driver/ppc-roptr.c:1
+// RUN: %clang -### -target powerpc-ibm-aix-xcoff -mroptr %s 2>&1 | FileCheck
%s
+// RUN: %clang -### -target powerpc-ibm-aix-xcoff -mroptr -mno-roptr %s 2>&1 |
\
hubert.reinterpretc
w2yehia added inline comments.
Comment at: clang/test/Driver/ppc-roptr.c:10
+// ROPTR-NOT: "-mroptr"
+// ROPTR-NOT: "-bforceimprw"
+
hubert.reinterpretcast wrote:
> Do we pass the option through to the LTO codegen when `-fprofile-generate` is
> used?
> We may ne
hubert.reinterpretcast added a subscriber: w2yehia.
hubert.reinterpretcast added inline comments.
Comment at: clang/test/CodeGen/PowerPC/aix-roptr.c:9-10
+// RUN: -S <%s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: not %clang_cc1 -triple=powerpc64le-unknown-li
hubert.reinterpretcast added inline comments.
Comment at: clang/docs/ReleaseNotes.rst:229-231
+- Introduced the ``-mroptr`` option to place constant objects with relocatable
+ address values in the ready-only data section. This option is intended to
+ be used with the ``-fdata-
qiongsiwu1 added a comment.
Ping for review. Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
htt
qiongsiwu1 updated this revision to Diff 501871.
qiongsiwu1 added a comment.
Updating `-fdata-sections` related comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Files:
clang/docs/ReleaseNotes.rs
qiongsiwu1 added a comment.
Gentle ping for review. @hubert.reinterpretcast Thanks so much!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
___
cfe-commits mail
qiongsiwu1 updated this revision to Diff 501264.
qiongsiwu1 added a comment.
Update lit test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Bas
qiongsiwu1 updated this revision to Diff 501211.
qiongsiwu1 added a comment.
Updating the release note and adding comments on the `-fdata-sections`
requirement.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D14
qiongsiwu1 added a comment.
Gentle ping for review. Thank you!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
___
cfe-commits mailing list
cfe-commits@lists.ll
qiongsiwu1 updated this revision to Diff 500298.
qiongsiwu1 added a comment.
> This accepts -mno-roptr for other platforms despite having no semantic
> functionality (e.g., it does not move variables to a different section for
> ELF codegen).
Thanks for catching this @hubert.reinterpretcast !!
hubert.reinterpretcast requested changes to this revision.
hubert.reinterpretcast added inline comments.
This revision now requires changes to proceed.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5251
+ if (Args.hasFlag(options::OPT_mroptr, options::OPT_mno_roptr, false)
stephenpeckham accepted this revision.
stephenpeckham added a comment.
This revision is now accepted and ready to land.
I don't have issues with this code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
qiongsiwu1 added a comment.
Gentle ping for review. Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
___
cfe-commits mailing list
cfe-commits@lists.llvm.
qiongsiwu1 updated this revision to Diff 498118.
qiongsiwu1 added a comment.
Rebase to resolve clang release note conflict.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144190/new/
https://reviews.llvm.org/D144190
Files:
clang/docs/ReleaseNotes
qiongsiwu1 created this revision.
qiongsiwu1 added a project: clang.
Herald added subscribers: ormris, kbarton, nemanjai.
Herald added a project: All.
qiongsiwu1 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
This patch adds clang options `-mroptr` and `-mno-rop
64 matches
Mail list logo