Skip to content

Revert "Reland [EquivClasses] Introduce members iterator-helper" #130380

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 8, 2025

Conversation

vitalybuka
Copy link
Collaborator

Reverts #130319

Multiple bot failures.

@vitalybuka vitalybuka requested a review from artagnon March 8, 2025 01:46
@vitalybuka vitalybuka added the skip-precommit-approval PR for CI feedback, not intended for review label Mar 8, 2025
@llvmbot llvmbot added llvm:analysis Includes value tracking, cost tables and constant folding llvm:adt and removed skip-precommit-approval PR for CI feedback, not intended for review labels Mar 8, 2025
@vitalybuka vitalybuka merged commit 5bc1667 into main Mar 8, 2025
9 of 15 checks passed
@llvmbot
Copy link
Member

llvmbot commented Mar 8, 2025

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-adt

Author: Vitaly Buka (vitalybuka)

Changes

Reverts llvm/llvm-project#130319

Multiple bot failures.


Full diff: https://github.com/llvm/llvm-project/pull/130380.diff

4 Files Affected:

  • (modified) llvm/include/llvm/ADT/EquivalenceClasses.h (-4)
  • (modified) llvm/lib/Analysis/LoopAccessAnalysis.cpp (+3-2)
  • (modified) llvm/lib/Analysis/VectorUtils.cpp (+3-3)
  • (modified) llvm/unittests/ADT/EquivalenceClassesTest.cpp (-14)
diff --git a/llvm/include/llvm/ADT/EquivalenceClasses.h b/llvm/include/llvm/ADT/EquivalenceClasses.h
index 345107c7777b0..4f98b84cf97d2 100644
--- a/llvm/include/llvm/ADT/EquivalenceClasses.h
+++ b/llvm/include/llvm/ADT/EquivalenceClasses.h
@@ -15,7 +15,6 @@
 #ifndef LLVM_ADT_EQUIVALENCECLASSES_H
 #define LLVM_ADT_EQUIVALENCECLASSES_H
 
-#include "llvm/ADT/iterator_range.h"
 #include <cassert>
 #include <cstddef>
 #include <cstdint>
@@ -179,9 +178,6 @@ class EquivalenceClasses {
   member_iterator member_end() const {
     return member_iterator(nullptr);
   }
-  iterator_range<member_iterator> members(iterator I) const {
-    return make_range(member_begin(I), member_end());
-  }
 
   /// findValue - Return an iterator to the specified value.  If it does not
   /// exist, end() is returned.
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 207f5417934e5..38ee82b77a946 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -527,8 +527,9 @@ void RuntimePointerChecking::groupChecks(
     // iteration order within an equivalence class member is only dependent on
     // the order in which unions and insertions are performed on the
     // equivalence class, the iteration order is deterministic.
-    for (const auto &MI : DepCands.members(LeaderI)) {
-      auto PointerI = PositionMap.find(MI.getPointer());
+    for (auto MI = DepCands.member_begin(LeaderI), ME = DepCands.member_end();
+         MI != ME; ++MI) {
+      auto PointerI = PositionMap.find(MI->getPointer());
       assert(PointerI != PositionMap.end() &&
              "pointer in equivalence class not found in PositionMap");
       for (unsigned Pointer : PointerI->second) {
diff --git a/llvm/lib/Analysis/VectorUtils.cpp b/llvm/lib/Analysis/VectorUtils.cpp
index c0bc451973c6e..91ba68fe03324 100644
--- a/llvm/lib/Analysis/VectorUtils.cpp
+++ b/llvm/lib/Analysis/VectorUtils.cpp
@@ -845,7 +845,7 @@ llvm::computeMinimumValueSizes(ArrayRef<BasicBlock *> Blocks, DemandedBits &DB,
 
   for (auto I = ECs.begin(), E = ECs.end(); I != E; ++I) {
     uint64_t LeaderDemandedBits = 0;
-    for (Value *M : ECs.members(I))
+    for (Value *M : llvm::make_range(ECs.member_begin(I), ECs.member_end()))
       LeaderDemandedBits |= DBits[M];
 
     uint64_t MinBW = llvm::bit_width(LeaderDemandedBits);
@@ -857,7 +857,7 @@ llvm::computeMinimumValueSizes(ArrayRef<BasicBlock *> Blocks, DemandedBits &DB,
     // indvars.
     // If we are required to shrink a PHI, abandon this entire equivalence class.
     bool Abort = false;
-    for (Value *M : ECs.members(I))
+    for (Value *M : llvm::make_range(ECs.member_begin(I), ECs.member_end()))
       if (isa<PHINode>(M) && MinBW < M->getType()->getScalarSizeInBits()) {
         Abort = true;
         break;
@@ -865,7 +865,7 @@ llvm::computeMinimumValueSizes(ArrayRef<BasicBlock *> Blocks, DemandedBits &DB,
     if (Abort)
       continue;
 
-    for (Value *M : ECs.members(I)) {
+    for (Value *M : llvm::make_range(ECs.member_begin(I), ECs.member_end())) {
       auto *MI = dyn_cast<Instruction>(M);
       if (!MI)
         continue;
diff --git a/llvm/unittests/ADT/EquivalenceClassesTest.cpp b/llvm/unittests/ADT/EquivalenceClassesTest.cpp
index c24c09d8a2815..70e161a03d988 100644
--- a/llvm/unittests/ADT/EquivalenceClassesTest.cpp
+++ b/llvm/unittests/ADT/EquivalenceClassesTest.cpp
@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/EquivalenceClasses.h"
-#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -67,19 +66,6 @@ TEST(EquivalenceClassesTest, TwoSets) {
         EXPECT_FALSE(EqClasses.isEquivalent(i, j));
 }
 
-TEST(EquivalenceClassesTest, MembersIterator) {
-  EquivalenceClasses<int> EC;
-  EC.unionSets(1, 2);
-  EC.insert(4);
-  EC.insert(5);
-  EC.unionSets(5, 1);
-  EXPECT_EQ(EC.getNumClasses(), 2u);
-
-  EquivalenceClasses<int>::iterator I = EC.findValue(EC.getLeaderValue(1));
-  EXPECT_THAT(EC.members(I), testing::ElementsAre(5, 1, 2));
-  EXPECT_EQ(EC.members(EC.end()).begin(), EC.member_end());
-}
-
 // Type-parameterized tests: Run the same test cases with different element
 // types.
 template <typename T> class ParameterizedTest : public testing::Test {};

@vitalybuka vitalybuka deleted the revert-130319-equivclass-members-it-reland branch March 8, 2025 01:46
@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 8, 2025

LLVM Buildbot has detected a new failure on builder arc-builder running on arc-worker while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/12787

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: 1200 seconds without output running [b'ninja', b'check-all'], attempting to kill
...
9.410 [84/18/20] cd /buildbot/worker/arc-folder/llvm-project/clang/bindings/python && /usr/local/bin/cmake -E env CLANG_NO_DEFAULT_CONFIG=1 CLANG_LIBRARY_PATH=/buildbot/worker/arc-folder/build/lib /usr/local/bin/python3.9 -m unittest discover
..........................................................................................................................................................
----------------------------------------------------------------------
Ran 154 tests in 1.407s

OK
9.450 [83/18/21] Linking CXX executable tools/clang/unittests/Driver/ClangDriverTests
10.506 [82/18/22] Linking CXX executable tools/clang/unittests/Analysis/FlowSensitive/ClangAnalysisFlowSensitiveTests
12.071 [81/18/23] Building CXX object unittests/ADT/CMakeFiles/ADTTests.dir/EquivalenceClassesTest.cpp.o
13.835 [80/18/24] Linking CXX executable unittests/ADT/ADTTests
command timed out: 1200 seconds without output running [b'ninja', b'check-all'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1281.590132

@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 8, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-win-x-aarch64 running on as-builder-2 while building llvm at step 9 "test-check-llvm".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/193/builds/6141

Here is the relevant piece of the build log for the reference
Step 9 (test-check-llvm) failure: Test just built components: check-llvm completed (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/50/87' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\buildbot\as-builder-2\x-aarch64\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-14968-50-87.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=87 GTEST_SHARD_INDEX=50 C:\buildbot\as-builder-2\x-aarch64\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\buildbot\as-builder-2\x-aarch64\build\unittests\Support\.\SupportTests.exe --gtest_filter=Caching.WriteAfterCommit
--
C:\buildbot\as-builder-2\x-aarch64\llvm-project\llvm\unittests\Support\Caching.cpp(103): error: Value of: AddStream
  Actual: false
Expected: true


C:\buildbot\as-builder-2\x-aarch64\llvm-project\llvm\unittests\Support\Caching.cpp:103
Value of: AddStream
  Actual: false
Expected: true



********************


@artagnon
Copy link
Contributor

artagnon commented Mar 8, 2025

Thanks @vitalybuka: sorry, I was asleep, and couldn't do this myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:adt llvm:analysis Includes value tracking, cost tables and constant folding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants