Skip to content

Commit 662cb2b

Browse files
committed
address reviewer comments
Signed-off-by: Bangtian Liu <liubangtian@gmail.com>
1 parent 22deac5 commit 662cb2b

2 files changed

Lines changed: 67 additions & 32 deletions

File tree

README.md

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -181,23 +181,7 @@ Pass custom IREE compiler flags using:
181181
- **C++ driver**: `FUSILLI_EXTRA_COMPILER_FLAGS` environment variable
182182
- **Python wrapper**: `--Xiree-compile` flag (which sets the environment variable internally)
183183

184-
**Single flag examples:**
185-
186-
C++ driver:
187-
```shell
188-
FUSILLI_EXTRA_COMPILER_FLAGS="--iree-opt-level=O3" \
189-
build/bin/benchmarks/fusilli_benchmark_driver --iter 100 \
190-
matmul -M 8192 -N 2048 -K 4096 --transA --a_type bf16 --b_type bf16 --out_type bf16
191-
```
192-
193-
Python wrapper:
194-
```shell
195-
python benchmarks/run_benchmark.py \
196-
--Xiree-compile="--iree-opt-level=O3" \
197-
-f commands.txt -o results.csv
198-
```
199-
200-
**Multiple flags:**
184+
**Examples:**
201185

202186
C++ driver (space-separated in one string):
203187
```shell

tests/test_compile_session.cpp

Lines changed: 66 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#include <algorithm>
1616
#include <filesystem>
17+
#include <fstream>
1718
#include <string>
1819
#include <vector>
1920

@@ -530,10 +531,31 @@ TEST_CASE("CompileSession::addFlag with tuning spec path",
530531
FUSILLI_REQUIRE_ASSIGN(CompileSession session,
531532
context->createSession(handle));
532533

533-
std::string tuningSpecPath = "/tmp/test_tuning_spec.mlir";
534+
// Create tuning spec in temp directory (outside cache) to avoid cache
535+
// validation issues. IREE registers tuning spec paths globally, so we need
536+
// a real file (not a dummy path).
537+
std::filesystem::path tempDir =
538+
std::filesystem::temp_directory_path() / "fusilli_test_tuning_specs";
539+
std::filesystem::create_directories(tempDir);
540+
std::filesystem::path tuningSpecPath = tempDir / "test1_tuning_spec.mlir";
534541

535-
ErrorObject result =
536-
session.addFlag("--iree-codegen-tuning-spec-path=" + tuningSpecPath);
542+
std::string tuningSpecContent = R"(
543+
module attributes {transform.with_named_sequence} {
544+
transform.named_sequence @__kernel_config(%arg0: !transform.any_op {transform.consumed})
545+
-> !transform.any_op attributes {iree_codegen.tuning_spec_entrypoint} {
546+
transform.yield %arg0 : !transform.any_op
547+
}
548+
}
549+
)";
550+
551+
// Write tuning spec file to temp directory.
552+
std::ofstream ofs(tuningSpecPath);
553+
REQUIRE(ofs.is_open());
554+
ofs << tuningSpecContent;
555+
ofs.close();
556+
557+
ErrorObject result = session.addFlag("--iree-codegen-tuning-spec-path=" +
558+
tuningSpecPath.generic_string());
537559
FUSILLI_REQUIRE_OK(result);
538560

539561
const std::vector<std::string> &args = session.getArgs();
@@ -542,6 +564,10 @@ TEST_CASE("CompileSession::addFlag with tuning spec path",
542564
return arg.find("--iree-codegen-tuning-spec-path") != std::string::npos;
543565
});
544566
REQUIRE(hasTuningSpecFlag);
567+
568+
// Note: We don't clean up the temp file here because IREE has process-wide
569+
// caching that references this path. Deleting it would break subsequent
570+
// tests. The temp directory will be cleaned up by the OS or test runner.
545571
}
546572

547573
TEST_CASE("CompileSession::compile with tuning spec",
@@ -553,10 +579,12 @@ TEST_CASE("CompileSession::compile with tuning spec",
553579
FUSILLI_REQUIRE_ASSIGN(CompileSession session,
554580
context->createSession(handle));
555581

556-
// Create a minimal no-op tuning spec.
557-
FUSILLI_REQUIRE_ASSIGN(
558-
CacheFile tuningSpec,
559-
CacheFile::create(kGraphName, "test_tuning_spec.mlir", /*remove=*/true));
582+
// Create tuning spec in temp directory (outside cache) to avoid cache
583+
// validation issues.
584+
std::filesystem::path tempDir =
585+
std::filesystem::temp_directory_path() / "fusilli_test_tuning_specs";
586+
std::filesystem::create_directories(tempDir);
587+
std::filesystem::path tuningSpecPath = tempDir / "test2_tuning_spec.mlir";
560588

561589
std::string tuningSpecContent = R"(
562590
module attributes {transform.with_named_sequence} {
@@ -566,25 +594,48 @@ module attributes {transform.with_named_sequence} {
566594
}
567595
}
568596
)";
569-
FUSILLI_REQUIRE_OK(tuningSpec.write(tuningSpecContent));
597+
598+
// Write tuning spec file to temp directory.
599+
std::ofstream ofs(tuningSpecPath);
600+
REQUIRE(ofs.is_open());
601+
ofs << tuningSpecContent;
602+
ofs.close();
570603

571604
FUSILLI_REQUIRE_OK(session.addFlag("--iree-codegen-tuning-spec-path=" +
572-
tuningSpec.path.string()));
605+
tuningSpecPath.generic_string()));
606+
607+
// Create input/output files in cache (these will auto-cleanup with
608+
// remove=true).
609+
std::string graphName = "test_compile_session_with_tuning_spec";
573610
FUSILLI_REQUIRE_ASSIGN(
574611
CacheFile input,
575-
CacheFile::create(kGraphName, "input_with_spec.mlir", /*remove=*/true));
612+
CacheFile::create(graphName, "input.mlir", /*remove=*/true));
576613
FUSILLI_REQUIRE_ASSIGN(
577614
CacheFile output,
578-
CacheFile::create(kGraphName, "output_with_spec.vmfb", /*remove=*/true));
579-
580-
std::string mlirContent = getSimpleMLIRModule();
615+
CacheFile::create(graphName, "output.vmfb", /*remove=*/true));
616+
617+
// Use unique MLIR content to avoid cache sharing with other tests.
618+
std::string mlirContent = R"(
619+
module {
620+
func.func @tuning_spec_test(%arg0: tensor<3x3xf32>, %arg1: tensor<3x3xf32>) -> tensor<3x3xf32> {
621+
%0 = arith.mulf %arg0, %arg1 : tensor<3x3xf32>
622+
return %0 : tensor<3x3xf32>
623+
}
624+
}
625+
)";
581626
FUSILLI_REQUIRE_OK(input.write(mlirContent));
582627

583-
ErrorObject compileResult =
584-
session.compile(input.path.string(), output.path.string());
628+
ErrorObject compileResult = session.compile(input.path.generic_string(),
629+
output.path.generic_string());
585630
FUSILLI_REQUIRE_OK(compileResult);
586631
REQUIRE(std::filesystem::exists(output.path));
587632
REQUIRE(std::filesystem::file_size(output.path) > 0);
588633

634+
// Clean up the cache directory. Input/output files have remove=true but their
635+
// destructors haven't run yet, so use remove_all to clean the directory.
589636
std::filesystem::remove_all(input.path.parent_path());
637+
638+
// Note: We don't clean up the temp tuning spec file here because IREE has
639+
// process-wide caching that references this path. Deleting it would break
640+
// subsequent tests. The temp directory will be cleaned up by the OS.
590641
}

0 commit comments

Comments
 (0)