Add build flag for out-of-tree builds #1403
Conversation
| if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) | ||
| option(TORCH_MLIR_OOT_BUILD "Specifies an out of tree build" OFF) | ||
|
|
||
| if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR OR TORCH_MLIR_OOT_BUILD) |
There was a problem hiding this comment.
Can we reliably detect this without the option? I thought that mlir-hlo or torch-mlir-external-dialects had a way to do this.
cc @marbre
There was a problem hiding this comment.
mlir-hlo has MHLO_BUILD_EMBEDDED which, as far as I can tell, serves the same purpose and we use it in torch-mlir when building MHLO. For torch-mlir-external-dialects it uses the MLIR_FOUND flag which could work instead, but I don't know if that would affect other torch-mlir builds.
There was a problem hiding this comment.
Can you rename this to TORCH_MLIR_BUILD_EMBEDDED for consistency?
Also, generally you should not merge PR's without resolving all conversations.
There was a problem hiding this comment.
Sure. Was there an unresolved conversation here though?
There was a problem hiding this comment.
Yes, this one is marked unresolved.
There was a problem hiding this comment.
Please open an issue to track that feature request and we can see if there is a more general interest.
There was a problem hiding this comment.
Can we at least rename the flag OUT_OF_TREE instead of OOT?
There was a problem hiding this comment.
I opened #1416. Apologies if I've been difficult here. How should I avoid this in the future, as having code-review level problems show up in a merged PR is definitely not something that should be happening often?
There was a problem hiding this comment.
In this case since I'm in a different time zone (CEST), it is somewhat expected for this to happen and we just have to be open to post-commit review. For "major" things like build flags letting it sit until all time zones can have a chance to chime in would be good.
I have to apologize, I didn't see that this was already merged when I left my comment.
There was a problem hiding this comment.
Can we reliably detect this without the option? I thought that mlir-hlo or torch-mlir-external-dialects had a way to do this.
cc @marbre
Sry, i am currently oof (and sick), but I can take a look after I return. I think we can handle this more consistently across the projects.
…lvm#1403) Use builder based interface to generate Krnl loops for Scan.cpp and Pooling.cpp. Signed-off-by: Whitney Tsang whitneyt@ca.ibm.com
The modification to use the builder based interface to generate Krnl loop is completed (llvm#1250, llvm#1283, llvm#1285, llvm#1292, llvm#1293, llvm#1303, llvm#1308, llvm#1314, llvm#1316, llvm#1317, llvm#1326, llvm#1403), and BuildKrnlLoop is no longer needed. Signed-off-by: Whitney Tsang whitneyt@ca.ibm.com
Needed for external builds when we have an out-of-tree build (e.g. onnx-mlir)