onnx-mlir
onnx-mlir copied to clipboard
-mtriple, TARGET_TRIPLE & LLVM_HOST_TRIPLE
llvm keeps track of a couple of triples in cmake for each compilation: TARGET_TRIPLE
(which is set by using LLVM_DEFAULT_TARGET_TRIPLE
, for example) & LLVM_HOST_TRIPLE
(which is set by the llvm cmake based on the host). When TARGET_TRIPLE exists, several of the tools such as llc
will default to using that triple when invoked and that can be overwritten by passing -mtriple
to the tool. Since some of the llvm tests only work correctly for some triples, they can either be disabled for other triples/platforms/targets/etc. or they explicitly pass a triple to the tools, so that they can be run whenever that triple is supported.
onnx-mlir
provides the option to pass -mtriple
which is then used for each of the tools it invokes (such as llc
) and when this is not present, defaults to whatever the tools default to by not providing an option. The tests themselves do not specify a triple either, so they always run for the default. (I realize that some of the tests such as the backend tests have the option to specify a test triple in the environment, but they themselves do not do so).
This is all well and good in theory for onnx-mlir
, but it means that the tests will (often) fail if the llvm compilation is done when specifying a default target triple that is not the host triple. Ideally, the tests will run for the host triple (or whatever the "correct" triple is) regardless of whether the build of llvm specified a default target triple.
This can be done in a couple of different ways:
- Modify the cmake targets for the tests to specify a default target triple that is expected to pass
- Modify
onnx-mlir
to default to theLLVM_HOST_TRIPLE
when-mtriple
is not specified.
I think 1) is probably the correct approach here, but 2) would also allow the tests to pass and provide a deterministic default.
Currently, when using cmake backend, it eventually goes to test.py which uses a TEST_MTRIPLE and TEST_MCPU to use a default triple. I like your suggestion for 2) better than the current solution that works only for the backend test.
I don't have a preference for 1 vs 2 above. My only suggestion is not to use the LLVM_HOST_TRIPLE
name because that would imply it also work for clang,... I would suggest ONNX_MLIR_TRIPLE
and ONNX_MLIR_MCPU
(which also need to be set).
I don't have the experience to do much CMAKE engineering, can work on 2 if the consensus is to go along with that.
The change is actually fairly simple and I have it working locally, so I can send it for review. If you like 2) better than 1), I'll send it out soonish.
Send what you have for review