[llvm] 1ad4527 - Revert "llvm-reduce: Try to kill parallel workitems once we have a result."
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 11 08:11:29 PST 2023
Author: Matt Arsenault
Date: 2023-01-11T11:11:23-05:00
New Revision: 1ad4527801e59d218615c5a73e2ac04904b49235
URL: https://github.com/llvm/llvm-project/commit/1ad4527801e59d218615c5a73e2ac04904b49235
DIFF: https://github.com/llvm/llvm-project/commit/1ad4527801e59d218615c5a73e2ac04904b49235.diff
LOG: Revert "llvm-reduce: Try to kill parallel workitems once we have a result."
This reverts commit 4f575620d51032cf98424c9defafe4dfc8d66f45.
I realized the test wasn't very good and when fixed, shows the
reduction doesn't work correctly. Revert the change and keep the fixed
version of the test.
Added:
llvm/test/tools/llvm-reduce/Inputs/sleep-and-check-stores.py
Modified:
llvm/test/tools/llvm-reduce/parallel-workitem-kill.ll
llvm/tools/llvm-reduce/TestRunner.cpp
llvm/tools/llvm-reduce/TestRunner.h
llvm/tools/llvm-reduce/deltas/Delta.cpp
llvm/tools/llvm-reduce/llvm-reduce.cpp
Removed:
llvm/test/tools/llvm-reduce/Inputs/sleep.py
################################################################################
diff --git a/llvm/test/tools/llvm-reduce/Inputs/sleep-and-check-stores.py b/llvm/test/tools/llvm-reduce/Inputs/sleep-and-check-stores.py
new file mode 100755
index 0000000000000..8df093e667b2a
--- /dev/null
+++ b/llvm/test/tools/llvm-reduce/Inputs/sleep-and-check-stores.py
@@ -0,0 +1,28 @@
+#!/bin/python
+
+import time
+import sys
+
+sleep_seconds = int(sys.argv[1])
+num_stores = int(sys.argv[2])
+file_input = sys.argv[3]
+
+try:
+ input = open(file_input, "r")
+except Exception as err:
+ print(err, file=sys.stderr)
+ sys.exit(1)
+
+InterestingStores = 0
+for line in input:
+ if "store" in line:
+ InterestingStores += 1
+
+print("Interesting stores ", InterestingStores, " sleeping ", sleep_seconds)
+time.sleep(sleep_seconds)
+
+
+if InterestingStores > num_stores:
+ sys.exit(0) # interesting!
+
+sys.exit(1) # IR isn't interesting
diff --git a/llvm/test/tools/llvm-reduce/Inputs/sleep.py b/llvm/test/tools/llvm-reduce/Inputs/sleep.py
deleted file mode 100755
index 8fd230ab74eb6..0000000000000
--- a/llvm/test/tools/llvm-reduce/Inputs/sleep.py
+++ /dev/null
@@ -1,8 +0,0 @@
-#!/bin/python
-
-import time
-import sys
-
-sleep_seconds = int(sys.argv[1])
-time.sleep(sleep_seconds)
-
diff --git a/llvm/test/tools/llvm-reduce/parallel-workitem-kill.ll b/llvm/test/tools/llvm-reduce/parallel-workitem-kill.ll
index 8003548c7655e..da52a8bfdcc13 100644
--- a/llvm/test/tools/llvm-reduce/parallel-workitem-kill.ll
+++ b/llvm/test/tools/llvm-reduce/parallel-workitem-kill.ll
@@ -1,8 +1,14 @@
; REQUIRES: thread_support
-; RUN: llvm-reduce --process-poll-interval=1 -j 4 %s -o %t --delta-passes=instructions --test %python --test-arg %S/Inputs/sleep.py --test-arg 2
+; RUN: llvm-reduce -j 4 %s -o %t --delta-passes=instructions --test %python --test-arg %S/Inputs/sleep-and-check-stores.py --test-arg 1 --test-arg 5
; RUN: FileCheck %s < %t
; CHECK: define void @foo
+; CHECK: store
+; CHECK: store
+; CHECK: store
+; CHECK: store
+; CHECK: store
+; CHECK: store
; CHECK-NEXT: ret void
define void @foo(ptr %ptr) {
@@ -23,30 +29,5 @@ define void @foo(ptr %ptr) {
store i32 14, ptr %ptr
store i32 15, ptr %ptr
store i32 16, ptr %ptr
- store i32 17, ptr %ptr
- store i32 18, ptr %ptr
- store i32 19, ptr %ptr
- store i32 20, ptr %ptr
- store i32 21, ptr %ptr
- store i32 22, ptr %ptr
- store i32 23, ptr %ptr
- store i32 24, ptr %ptr
- store i32 25, ptr %ptr
- store i32 26, ptr %ptr
- store i32 27, ptr %ptr
- store i32 28, ptr %ptr
- store i32 29, ptr %ptr
- store i32 30, ptr %ptr
- store i32 31, ptr %ptr
- store i32 32, ptr %ptr
- store i32 33, ptr %ptr
- store i32 34, ptr %ptr
- store i32 35, ptr %ptr
- store i32 36, ptr %ptr
- store i32 37, ptr %ptr
- store i32 38, ptr %ptr
- store i32 39, ptr %ptr
- store i32 40, ptr %ptr
ret void
}
-
diff --git a/llvm/tools/llvm-reduce/TestRunner.cpp b/llvm/tools/llvm-reduce/TestRunner.cpp
index 1ec761a968f08..3a5483cdae1c2 100644
--- a/llvm/tools/llvm-reduce/TestRunner.cpp
+++ b/llvm/tools/llvm-reduce/TestRunner.cpp
@@ -18,12 +18,6 @@
using namespace llvm;
-extern cl::OptionCategory LLVMReduceOptions;
-static cl::opt<unsigned> PollInterval("process-poll-interval",
- cl::desc("child process wait polling"),
- cl::init(5), cl::Hidden,
- cl::cat(LLVMReduceOptions));
-
TestRunner::TestRunner(StringRef TestName,
const std::vector<std::string> &TestArgs,
std::unique_ptr<ReducerWorkItem> Program,
@@ -43,7 +37,7 @@ static constexpr std::array<std::optional<StringRef>, 3> NullRedirects;
/// Runs the interestingness test, passes file to be tested as first argument
/// and other specified test arguments after that.
-int TestRunner::run(StringRef Filename, const std::atomic<bool> &Killed) const {
+int TestRunner::run(StringRef Filename) const {
std::vector<StringRef> ProgramArgs;
ProgramArgs.push_back(TestName);
@@ -53,13 +47,13 @@ int TestRunner::run(StringRef Filename, const std::atomic<bool> &Killed) const {
ProgramArgs.push_back(Filename);
std::string ErrMsg;
- bool ExecutionFailed;
- sys::ProcessInfo PI =
- sys::ExecuteNoWait(TestName, ProgramArgs, /*Env=*/std::nullopt,
- Verbose ? DefaultRedirects : NullRedirects,
- /*MemoryLimit=*/0, &ErrMsg, &ExecutionFailed);
- if (ExecutionFailed) {
+ int Result =
+ sys::ExecuteAndWait(TestName, ProgramArgs, /*Env=*/std::nullopt,
+ Verbose ? DefaultRedirects : NullRedirects,
+ /*SecondsToWait=*/0, /*MemoryLimit=*/0, &ErrMsg);
+
+ if (Result < 0) {
Error E = make_error<StringError>("Error running interesting-ness test: " +
ErrMsg,
inconvertibleErrorCode());
@@ -67,25 +61,7 @@ int TestRunner::run(StringRef Filename, const std::atomic<bool> &Killed) const {
exit(1);
}
- // Poll every few seconds, taking a break to check if we should try to kill
- // the process. We're trying to early exit on long running parallel reductions
- // once we know they don't matter.
- std::optional<unsigned> SecondsToWait(PollInterval);
- bool Polling = true;
- sys::ProcessInfo WaitPI;
-
- while (WaitPI.Pid == 0) { // Process has not changed state.
- WaitPI = sys::Wait(PI, SecondsToWait, &ErrMsg, nullptr, Polling);
- // TODO: This should probably be std::atomic_flag
- if (Killed) {
- // The current Program API does not have a way to directly kill, but we
- // can timeout after 0 seconds.
- SecondsToWait = 0;
- Polling = false;
- }
- }
-
- return !WaitPI.ReturnCode;
+ return !Result;
}
void TestRunner::setProgram(std::unique_ptr<ReducerWorkItem> P) {
diff --git a/llvm/tools/llvm-reduce/TestRunner.h b/llvm/tools/llvm-reduce/TestRunner.h
index 7af1096ff0a6b..128eede0feeae 100644
--- a/llvm/tools/llvm-reduce/TestRunner.h
+++ b/llvm/tools/llvm-reduce/TestRunner.h
@@ -33,7 +33,7 @@ class TestRunner {
/// Runs the interesting-ness test for the specified file
/// @returns 0 if test was successful, 1 if otherwise
- int run(StringRef Filename, const std::atomic<bool> &Killed) const;
+ int run(StringRef Filename) const;
/// Returns the most reduced version of the original testcase
ReducerWorkItem &getProgram() const { return *Program; }
diff --git a/llvm/tools/llvm-reduce/deltas/Delta.cpp b/llvm/tools/llvm-reduce/deltas/Delta.cpp
index 689d4fc6855d0..0def68c5f11c4 100644
--- a/llvm/tools/llvm-reduce/deltas/Delta.cpp
+++ b/llvm/tools/llvm-reduce/deltas/Delta.cpp
@@ -65,8 +65,7 @@ void writeBitcode(ReducerWorkItem &M, raw_ostream &OutStream);
void readBitcode(ReducerWorkItem &M, MemoryBufferRef Data, LLVMContext &Ctx,
StringRef ToolName);
-bool isReduced(ReducerWorkItem &M, const TestRunner &Test,
- const std::atomic<bool> &Killed) {
+bool isReduced(ReducerWorkItem &M, const TestRunner &Test) {
const bool UseBitcode = Test.inputIsBitcode() || TmpFilesAsBitcode;
SmallString<128> CurrentFilepath;
@@ -97,7 +96,7 @@ bool isReduced(ReducerWorkItem &M, const TestRunner &Test,
}
// Current Chunks aren't interesting
- return Test.run(CurrentFilepath, Killed);
+ return Test.run(CurrentFilepath);
}
/// Splits Chunks in half and prints them.
@@ -139,8 +138,7 @@ CheckChunk(const Chunk &ChunkToCheckForUninterestingness,
std::unique_ptr<ReducerWorkItem> Clone, const TestRunner &Test,
ReductionFunc ExtractChunksFromModule,
const DenseSet<Chunk> &UninterestingChunks,
- const std::vector<Chunk> &ChunksStillConsideredInteresting,
- const std::atomic<bool> &Killed) {
+ const std::vector<Chunk> &ChunksStillConsideredInteresting) {
// Take all of ChunksStillConsideredInteresting chunks, except those we've
// already deemed uninteresting (UninterestingChunks) but didn't remove
// from ChunksStillConsideredInteresting yet, and additionally ignore
@@ -180,7 +178,7 @@ CheckChunk(const Chunk &ChunkToCheckForUninterestingness,
errs() << "\n";
}
- if (!isReduced(*Clone, Test, Killed)) {
+ if (!isReduced(*Clone, Test)) {
// Program became non-reduced, so this chunk appears to be interesting.
if (Verbose)
errs() << "\n";
@@ -193,8 +191,7 @@ static SmallString<0> ProcessChunkFromSerializedBitcode(
Chunk &ChunkToCheckForUninterestingness, TestRunner &Test,
ReductionFunc ExtractChunksFromModule, DenseSet<Chunk> &UninterestingChunks,
std::vector<Chunk> &ChunksStillConsideredInteresting,
- SmallString<0> &OriginalBC, std::atomic<bool> &AnyReduced,
- const std::atomic<bool> &Killed) {
+ SmallString<0> &OriginalBC, std::atomic<bool> &AnyReduced) {
LLVMContext Ctx;
auto CloneMMM = std::make_unique<ReducerWorkItem>();
MemoryBufferRef Data(StringRef(OriginalBC), "<bc file>");
@@ -204,7 +201,7 @@ static SmallString<0> ProcessChunkFromSerializedBitcode(
if (std::unique_ptr<ReducerWorkItem> ChunkResult =
CheckChunk(ChunkToCheckForUninterestingness, std::move(CloneMMM),
Test, ExtractChunksFromModule, UninterestingChunks,
- ChunksStillConsideredInteresting, Killed)) {
+ ChunksStillConsideredInteresting)) {
raw_svector_ostream BCOS(Result);
writeBitcode(*ChunkResult, BCOS);
// Communicate that the task reduced a chunk.
@@ -245,7 +242,7 @@ void llvm::runDeltaPass(TestRunner &Test, ReductionFunc ExtractChunksFromModule,
assert(!verifyReducerWorkItem(Test.getProgram(), &errs()) &&
"input module is broken after counting chunks");
- assert(isReduced(Test.getProgram(), Test, std::atomic<bool>()) &&
+ assert(isReduced(Test.getProgram(), Test) &&
"input module no longer interesting after counting chunks");
#ifndef NDEBUG
@@ -293,11 +290,6 @@ void llvm::runDeltaPass(TestRunner &Test, ReductionFunc ExtractChunksFromModule,
writeBitcode(Test.getProgram(), BCOS);
}
- // If doing parallel reduction, signal to running workitem threads that we
- // no longer care about their results. They should try to kill the reducer
- // workitem process and exit.
- std::atomic<bool> Killed = false;
-
SharedTaskQueue TaskQueue;
for (auto I = ChunksStillConsideredInteresting.rbegin(),
E = ChunksStillConsideredInteresting.rend();
@@ -324,12 +316,11 @@ void llvm::runDeltaPass(TestRunner &Test, ReductionFunc ExtractChunksFromModule,
for (unsigned J = 0; J < NumInitialTasks; ++J) {
TaskQueue.emplace_back(ChunkThreadPool.async(
[J, I, &Test, &ExtractChunksFromModule, &UninterestingChunks,
- &ChunksStillConsideredInteresting, &OriginalBC, &AnyReduced,
- &Killed]() {
+ &ChunksStillConsideredInteresting, &OriginalBC, &AnyReduced]() {
return ProcessChunkFromSerializedBitcode(
*(I + J), Test, ExtractChunksFromModule,
UninterestingChunks, ChunksStillConsideredInteresting,
- OriginalBC, AnyReduced, Killed);
+ OriginalBC, AnyReduced);
}));
}
@@ -353,11 +344,11 @@ void llvm::runDeltaPass(TestRunner &Test, ReductionFunc ExtractChunksFromModule,
TaskQueue.emplace_back(ChunkThreadPool.async(
[&Test, &ExtractChunksFromModule, &UninterestingChunks,
&ChunksStillConsideredInteresting, &OriginalBC,
- &ChunkToCheck, &AnyReduced, &Killed]() {
+ &ChunkToCheck, &AnyReduced]() {
return ProcessChunkFromSerializedBitcode(
ChunkToCheck, Test, ExtractChunksFromModule,
UninterestingChunks, ChunksStillConsideredInteresting,
- OriginalBC, AnyReduced, Killed);
+ OriginalBC, AnyReduced);
}));
}
continue;
@@ -370,8 +361,6 @@ void llvm::runDeltaPass(TestRunner &Test, ReductionFunc ExtractChunksFromModule,
break;
}
- Killed = true;
-
// If we broke out of the loop, we still need to wait for everything to
// avoid race access to the chunk set.
//
@@ -386,7 +375,7 @@ void llvm::runDeltaPass(TestRunner &Test, ReductionFunc ExtractChunksFromModule,
*I,
cloneReducerWorkItem(Test.getProgram(), Test.getTargetMachine()),
Test, ExtractChunksFromModule, UninterestingChunks,
- ChunksStillConsideredInteresting, Killed);
+ ChunksStillConsideredInteresting);
}
if (!Result)
diff --git a/llvm/tools/llvm-reduce/llvm-reduce.cpp b/llvm/tools/llvm-reduce/llvm-reduce.cpp
index 8f064bc1e1199..07a04a6ccd8fa 100644
--- a/llvm/tools/llvm-reduce/llvm-reduce.cpp
+++ b/llvm/tools/llvm-reduce/llvm-reduce.cpp
@@ -101,8 +101,7 @@ static cl::opt<int>
static codegen::RegisterCodeGenFlags CGF;
-bool isReduced(ReducerWorkItem &M, const TestRunner &Test,
- const std::atomic<bool> &Killed);
+bool isReduced(ReducerWorkItem &M, const TestRunner &Test);
/// Turn off crash debugging features
///
@@ -218,7 +217,7 @@ int main(int Argc, char **Argv) {
// test, rather than evaluating the source IR directly. This is for the
// convenience of lit tests; the stripped out comments may have broken the
// interestingness checks.
- if (!isReduced(Tester.getProgram(), Tester, std::atomic<bool>())) {
+ if (!isReduced(Tester.getProgram(), Tester)) {
errs() << "\nInput isn't interesting! Verify interesting-ness test\n";
return 1;
}
More information about the llvm-commits
mailing list