[clang] [AIX][TOC] -mtocdata/-mno-tocdata fix non deterministic iteration order (PR #86840)
https://github.com/syzaara closed https://github.com/llvm/llvm-project/pull/86840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AIX][TOC] -mtocdata/-mno-tocdata fix non deterministic iteration order (PR #86840)
https://github.com/mandlebug approved this pull request. LGTM. Zaara pointed out offline that we can't rely on the input being sorted despite this change because you can invoke the front end directly with an argument list in unsorted order. https://github.com/llvm/llvm-project/pull/86840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AIX][TOC] -mtocdata/-mno-tocdata fix non deterministic iteration order (PR #86840)
https://github.com/mandlebug commented: It's unfortunate we have to ditch the StringSet because of the iteration order since the semantics of the option are the same regardless of the order the names are printed in. Now that we are constructing the list in sorted order we should change [these](https://github.com/llvm/llvm-project/blob/36e74cfdbde208e384c72bcb52ea638303fb7d67/clang/lib/Frontend/CompilerInstance.cpp#L1052) to assertions that they are already sorted. https://github.com/llvm/llvm-project/pull/86840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AIX][TOC] -mtocdata/-mno-tocdata fix non deterministic iteration order (PR #86840)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Zaara Syeda (syzaara)
Changes
Failure with testcase toc-conf.c observed when building with
LLVM_REVERSE_ITERATION=ON.
Changing from using llvm::StringSet to std::setllvm:StringRef to ensure
iteration order is deterministic. Note: the functionality of the feature does
not require a specific iteration order, however, this will allow testing to be
consistent.
>From llvm docs:
The advantages of std::set are that its iterators are stable (deleting or
inserting an element from the set does not affect iterators or pointers to
other elements) and that iteration over the set is guaranteed to be in sorted
order.
---
Full diff: https://github.com/llvm/llvm-project/pull/86840.diff
2 Files Affected:
- (modified) clang/lib/Driver/ToolChains/AIX.cpp (+3-3)
- (modified) clang/test/Driver/toc-conf.c (+1-1)
``diff
diff --git a/clang/lib/Driver/ToolChains/AIX.cpp
b/clang/lib/Driver/ToolChains/AIX.cpp
index 6e089903a3158a..7a62b0f9aec419 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -471,7 +471,7 @@ static void addTocDataOptions(const llvm::opt::ArgList
,
// the global setting of tocdata in TOCDataGloballyinEffect.
// Those that have the opposite setting to TOCDataGloballyinEffect, are added
// to ExplicitlySpecifiedGlobals.
- llvm::StringSet<> ExplicitlySpecifiedGlobals;
+ std::set ExplicitlySpecifiedGlobals;
for (const auto Arg :
Args.filtered(options::OPT_mtocdata_EQ, options::OPT_mno_tocdata_EQ)) {
TOCDataSetting ArgTocDataSetting =
@@ -486,7 +486,7 @@ static void addTocDataOptions(const llvm::opt::ArgList
,
ExplicitlySpecifiedGlobals.erase(Val);
}
- auto buildExceptionList = [](const llvm::StringSet<> ,
+ auto buildExceptionList = [](const std::set ,
const char *OptionSpelling) {
std::string Option(OptionSpelling);
bool IsFirst = true;
@@ -495,7 +495,7 @@ static void addTocDataOptions(const llvm::opt::ArgList
,
Option += ",";
IsFirst = false;
- Option += E.first();
+ Option += E.str();
}
return Option;
};
diff --git a/clang/test/Driver/toc-conf.c b/clang/test/Driver/toc-conf.c
index 80d92ee1a90b4b..7b2d5122ebc6ce 100644
--- a/clang/test/Driver/toc-conf.c
+++ b/clang/test/Driver/toc-conf.c
@@ -23,7 +23,7 @@ void func() {
// CHECK-CONF1-NOT: warning:
// CHECK-CONF1: "-cc1"{{.*}}" "-mno-tocdata"
-// CHECK-CONF1: "-mtocdata=g2,g1"
+// CHECK-CONF1: "-mtocdata=g1,g2"
// CHECK-CONF2-NOT: warning:
// CHECK-CONF2: "-cc1"{{.*}}" "-mtocdata"
``
https://github.com/llvm/llvm-project/pull/86840
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AIX][TOC] -mtocdata/-mno-tocdata fix non deterministic iteration order (PR #86840)
https://github.com/syzaara created
https://github.com/llvm/llvm-project/pull/86840
Failure with testcase toc-conf.c observed when building with
LLVM_REVERSE_ITERATION=ON.
Changing from using llvm::StringSet to std::set to ensure
iteration order is deterministic. Note: the functionality of the feature does
not require a specific iteration order, however, this will allow testing to be
consistent.
>From llvm docs:
The advantages of std::set are that its iterators are stable (deleting or
inserting an element from the set does not affect iterators or pointers to
other elements) and that iteration over the set is guaranteed to be in sorted
order.
>From 28293a34d459651a2d0d7e7783c47f782914a776 Mon Sep 17 00:00:00 2001
From: Zaara Syeda
Date: Wed, 27 Mar 2024 13:35:29 -0400
Subject: [PATCH] [AIX][TOC] -mtocdata/-mno-tocdata fix non deterministic
iteration order
Failure with testcase toc-conf.c observed when building with
LLVM_REVERSE_ITERATION=ON.
Changing from using llvm::StringSet to std::set
to ensure iteration order is deterministic. Note: the functionality
of the feature does not require a specific iteration order, however,
this will allow testing to be consistent.
>From llvm docs:
The advantages of std::set are that its iterators are stable
(deleting or inserting an element from the set does not affect
iterators or pointers to other elements) and that iteration over
the set is guaranteed to be in sorted order.
---
clang/lib/Driver/ToolChains/AIX.cpp | 6 +++---
clang/test/Driver/toc-conf.c| 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/AIX.cpp
b/clang/lib/Driver/ToolChains/AIX.cpp
index 6e089903a3158a..7a62b0f9aec419 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -471,7 +471,7 @@ static void addTocDataOptions(const llvm::opt::ArgList
,
// the global setting of tocdata in TOCDataGloballyinEffect.
// Those that have the opposite setting to TOCDataGloballyinEffect, are added
// to ExplicitlySpecifiedGlobals.
- llvm::StringSet<> ExplicitlySpecifiedGlobals;
+ std::set ExplicitlySpecifiedGlobals;
for (const auto Arg :
Args.filtered(options::OPT_mtocdata_EQ, options::OPT_mno_tocdata_EQ)) {
TOCDataSetting ArgTocDataSetting =
@@ -486,7 +486,7 @@ static void addTocDataOptions(const llvm::opt::ArgList
,
ExplicitlySpecifiedGlobals.erase(Val);
}
- auto buildExceptionList = [](const llvm::StringSet<> ,
+ auto buildExceptionList = [](const std::set ,
const char *OptionSpelling) {
std::string Option(OptionSpelling);
bool IsFirst = true;
@@ -495,7 +495,7 @@ static void addTocDataOptions(const llvm::opt::ArgList
,
Option += ",";
IsFirst = false;
- Option += E.first();
+ Option += E.str();
}
return Option;
};
diff --git a/clang/test/Driver/toc-conf.c b/clang/test/Driver/toc-conf.c
index 80d92ee1a90b4b..7b2d5122ebc6ce 100644
--- a/clang/test/Driver/toc-conf.c
+++ b/clang/test/Driver/toc-conf.c
@@ -23,7 +23,7 @@ void func() {
// CHECK-CONF1-NOT: warning:
// CHECK-CONF1: "-cc1"{{.*}}" "-mno-tocdata"
-// CHECK-CONF1: "-mtocdata=g2,g1"
+// CHECK-CONF1: "-mtocdata=g1,g2"
// CHECK-CONF2-NOT: warning:
// CHECK-CONF2: "-cc1"{{.*}}" "-mtocdata"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
