Skip to content

[mlir][cmake] Add missing MLIRTestDialect dependency #125004

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
Jan 31, 2025

Conversation

dcaballe
Copy link
Contributor

Hopefully this will fix the flaky build issue that we have in one of the bots: https://lab.llvm.org/buildbot/#/builders/50/builds/9532

Hopefully this will fix the flaky build issue that we have in one of the
bots: https://lab.llvm.org/buildbot/#/builders/50/builds/9532
@llvmbot llvmbot added the mlir label Jan 30, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 30, 2025

@llvm/pr-subscribers-mlir

Author: Diego Caballero (dcaballe)

Changes

Hopefully this will fix the flaky build issue that we have in one of the bots: https://lab.llvm.org/buildbot/#/builders/50/builds/9532


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

1 Files Affected:

  • (modified) mlir/test/lib/IR/CMakeLists.txt (+4)
diff --git a/mlir/test/lib/IR/CMakeLists.txt b/mlir/test/lib/IR/CMakeLists.txt
index e5416da70d5008..eeb9cf1e34fc8c 100644
--- a/mlir/test/lib/IR/CMakeLists.txt
+++ b/mlir/test/lib/IR/CMakeLists.txt
@@ -27,7 +27,11 @@ add_mlir_library(MLIRTestIR
   TestVisitorsGeneric.cpp
 
   EXCLUDE_FROM_LIBMLIR
+
+  DEPENDS
+  MLIRTestDialect
   )
+
 mlir_target_link_libraries(MLIRTestIR PUBLIC
   MLIRPass
   MLIRBytecodeReader

@dcaballe dcaballe merged commit 72b73c9 into llvm:main Jan 31, 2025
10 checks passed
DavidSpickett added a commit that referenced this pull request Feb 3, 2025
That Linaro has been seeing on our dylib bot:
https://lab.llvm.org/staging/#/builders/171/builds/79

In file included from /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/TestPassManager.cpp:10:
/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/../Dialect/Test/TestOps.h:148:10: fatal error: 'TestOps.h.inc' file not found
  148 | #include "TestOps.h.inc"
      |          ^~~~~~~~~~~~~~~

Reported in #124335 and
#124485.

I think this was caused by #123910,
and so I've followed what #125004 did,
which seems to be a follow up to that. Same kind of error.

I was not able to reproduce the failure locally, but dylib and
normal builds are ok with this change so I will push it and monitor
the bot's results for a few days.
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Feb 3, 2025
That Linaro has been seeing on our dylib bot:
https://lab.llvm.org/staging/#/builders/171/builds/79

In file included from /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/TestPassManager.cpp:10:
/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/../Dialect/Test/TestOps.h:148:10: fatal error: 'TestOps.h.inc' file not found
  148 | #include "TestOps.h.inc"
      |          ^~~~~~~~~~~~~~~

Reported in llvm/llvm-project#124335 and
llvm/llvm-project#124485.

I think this was caused by llvm/llvm-project#123910,
and so I've followed what llvm/llvm-project#125004 did,
which seems to be a follow up to that. Same kind of error.

I was not able to reproduce the failure locally, but dylib and
normal builds are ok with this change so I will push it and monitor
the bot's results for a few days.
nikic added a commit to nikic/llvm-project that referenced this pull request Feb 4, 2025
This is a test library which is not part of libMLIR, so it should
use normal LINK_LIBS instead of mlir_target_link_libraries.

This fixes an issue introduced in llvm#123910 and follows up on the
fix in llvm#125004, which added the library to DEPENDS, which is not
sufficient.
nikic added a commit that referenced this pull request Feb 5, 2025
This is a test library which is not part of libMLIR, so it should use
normal LINK_LIBS instead of mlir_target_link_libraries.

This fixes an issue introduced in #123910 and follows up on the fix in
#125004, which added the library to DEPENDS, which is not sufficient.
DavidSpickett pushed a commit to DavidSpickett/llvm-project that referenced this pull request Feb 5, 2025
This cherry picks
[mlir][cmake] Add missing MLIRTestDialect dependency (llvm#125004) (72b73c9)
and
[mlir] Fix build race condition in Pass Manager tests (d906da5)
to the 20.x release branch.

Both of these are addressing issues that started with
llvm#123910, which is already on the 20.x branch.

In each case the cherry-picked changes correct problems that would otherwise be
a race condition in certain builds. Linaro noticed this on our flang dylib (shared library)
build bot.

Failures look like:
In file included from /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/TestPassManager.cpp:10:
/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/../Dialect/Test/TestOps.h:148:10: fatal error: 'TestOps.h.inc' file not found
  148 | #include "TestOps.h.inc"
      |          ^~~~~~~~~~~~~~~

We have tested these changes on the buildbot for the last 2 days and had no problems.
Whereas before it was failing maybe 1 in 10 builds, enough that multiple people
in the community noticed it.

Reported in llvm#124485 and
llvm#124335.

Co-authored-by: Diego Caballero <[email protected]>
nikic added a commit to nikic/llvm-project that referenced this pull request Feb 5, 2025
This is a test library which is not part of libMLIR, so it should
use normal LINK_LIBS instead of mlir_target_link_libraries.

This fixes an issue introduced in llvm#123910 and follows up on the
fix in llvm#125004, which added the library to DEPENDS, which is not
sufficient.
nikic added a commit to nikic/llvm-project that referenced this pull request Feb 11, 2025
This is a test library which is not part of libMLIR, so it should
use normal LINK_LIBS instead of mlir_target_link_libraries.

This fixes an issue introduced in llvm#123910 and follows up on the
fix in llvm#125004, which added the library to DEPENDS, which is not
sufficient.
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
That Linaro has been seeing on our dylib bot:
https://lab.llvm.org/staging/#/builders/171/builds/79

In file included from /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/TestPassManager.cpp:10:
/home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/mlir/test/lib/Pass/../Dialect/Test/TestOps.h:148:10: fatal error: 'TestOps.h.inc' file not found
  148 | #include "TestOps.h.inc"
      |          ^~~~~~~~~~~~~~~

Reported in llvm#124335 and
llvm#124485.

I think this was caused by llvm#123910,
and so I've followed what llvm#125004 did,
which seems to be a follow up to that. Same kind of error.

I was not able to reproduce the failure locally, but dylib and
normal builds are ok with this change so I will push it and monitor
the bot's results for a few days.
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
This is a test library which is not part of libMLIR, so it should use
normal LINK_LIBS instead of mlir_target_link_libraries.

This fixes an issue introduced in llvm#123910 and follows up on the fix in
llvm#125004, which added the library to DEPENDS, which is not sufficient.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants