Skip to content
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

[cmake build] MhloDialect target no longer carries correct binary include dir #57

Closed
christopherbate opened this issue Jan 25, 2023 · 5 comments

Comments

@christopherbate
Copy link

christopherbate commented Jan 25, 2023

After this latest change to mlir-hlo/mhlo/CMakeLists.txt, the MhloDialect target no longer has the correct binary include directories attached to its INTERFACE_INCLUDE_DIRECTORIES. I now have to do the following in my downstream project when building mlir-hlo via the "LLVM External Project" route:

get_target_property(mhlo_includes_ MhloDialect INTERFACE_INCLUDE_DIRECTORIES)
list(APPEND mhlo_includes_ $<BUILD_INTERFACE:${LLVM_BINARY_DIR}/tools/mlir-hlo>)
set_target_properties(MhloDialect PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${mhlo_includes_}")

Otherwise, the build will fail on downstream targets that depend on MhloDialect. The compiler will say:

 fatal error: 'mhlo/IR/hlo_ops_enums.h.inc' file not found

Note that this is different from #52 (which probably can now be closed).

Edit: fixed the workaround

@burmako
Copy link
Contributor

burmako commented Jan 25, 2023

Hi @christopherbate! Thank you for reporting.
@ddunl Can you take a look please?

@ddunl
Copy link
Contributor

ddunl commented Jan 26, 2023

Thanks for bringing this to my attention! @christopherbate can you let me know if d391887 fixes this?

@christopherbate
Copy link
Author

@ddunl Thanks for your response. This actually doesn't fix it (in LLVM External Project mode).

I added a print statement like this:

message(WARNING "MHLO DIRS: ${MLIR_HLO_SOURCE_DIR} ${MLIR_HLO_GEN_INCLUDE_DIR}")
target_include_directories(MhloDialect
  PUBLIC
  $<BUILD_INTERFACE:${MLIR_HLO_SOURCE_DIR}>
  $<BUILD_INTERFACE:${MLIR_HLO_GEN_INCLUDE_DIR}>
)

and MLIR_HLO_GEN_INCLUDE_DIR points to something like "build/llvm/tools/mlir-hlo/include" but actually the build tree under build/llvm/tools/mlir-hlo doesn't have an include directory.

@ddunl
Copy link
Contributor

ddunl commented Mar 1, 2023

Ah I was clearly not being careful enough when I made that change. I'll test a fix more thoroughly tomorrow. So sorry this is taking so long to get resolved!

@ddunl
Copy link
Contributor

ddunl commented Mar 3, 2023

Woops, didn't intend for this to be closed. Let me know if a170d81 fixes this. I had a hard time finding a project using LLVM External Project mode, but I was able to replicate the faulty path using that message(...) line, and it appears to be fixed. Sorry about this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants