[CMake] Only set LLVM_DEFAULT_TARGET_TRIPLE to LLVM_HOST_TRIPLE when native target is enabled

[python3kgae]

Summary

This is for case when native target like X86 is not in LLVM_TARGETS_TO_BUILD.
Right now LLVM_DEFAULT_TARGET_TRIPLE is set to LLVM_HOST_TRIPLE even when native target is not enabled,
As a result, many lit tests will fail because default_triple is set for lit test but not enabled when build LLVM.

[python3kgae]

Gentle ping.

[smeenai]

Sorry for the slow review.

Does this mean that LLVM_DEFAULT_TARGET_TRIPLE will now be empty when the native target isn't enabled (since LLVM_DEFAULT_TARGET_TRIPLE_default won't be set)? Does that cause any problems?

[python3kgae]

In D134972#3844340, @smeenai wrote:

Sorry for the slow review.

Does this mean that LLVM_DEFAULT_TARGET_TRIPLE will now be empty when the native target isn't enabled (since LLVM_DEFAULT_TARGET_TRIPLE_default won't be set)? Does that cause any problems?

Thanks for reviewing.
Yes. Now LLVM_DEFAULT_TARGET_TRIPLE will be empty when the native target is not enabled.
check-llvm passed with this change (It fails when the native target is not enabled without this change. I only tested Linux build on x86 though.)

[smeenai]

In D134972#3844420, @python3kgae wrote:

In D134972#3844340, @smeenai wrote:

Sorry for the slow review.

Does this mean that LLVM_DEFAULT_TARGET_TRIPLE will now be empty when the native target isn't enabled (since LLVM_DEFAULT_TARGET_TRIPLE_default won't be set)? Does that cause any problems?

Thanks for reviewing.
Yes. Now LLVM_DEFAULT_TARGET_TRIPLE will be empty when the native target is not enabled.
check-llvm passed with this change (It fails when the native target is not enabled without this change. I only tested Linux build on x86 though.)

What about check-clang? What does clang do in this case if you try a compile without specifying an explicit triple?

Conceptually, requiring the user to explicitly pick a default target triple from one of the backends they have enabled seems cleaner than having it be empty to me. We'd error out if you don't have the native target enabled and didn't explicitly specify LLVM_DEFAULT_TARGET_TRIPLE. Would that work for you?

[python3kgae]

In D134972#3847120, @smeenai wrote:

In D134972#3844420, @python3kgae wrote:

In D134972#3844340, @smeenai wrote:

Sorry for the slow review.

Does this mean that LLVM_DEFAULT_TARGET_TRIPLE will now be empty when the native target isn't enabled (since LLVM_DEFAULT_TARGET_TRIPLE_default won't be set)? Does that cause any problems?

Thanks for reviewing.
Yes. Now LLVM_DEFAULT_TARGET_TRIPLE will be empty when the native target is not enabled.
check-llvm passed with this change (It fails when the native target is not enabled without this change. I only tested Linux build on x86 though.)

What about check-clang? What does clang do in this case if you try a compile without specifying an explicit triple?

Conceptually, requiring the user to explicitly pick a default target triple from one of the backends they have enabled seems cleaner than having it be empty to me. We'd error out if you don't have the native target enabled and didn't explicitly specify LLVM_DEFAULT_TARGET_TRIPLE. Would that work for you?

check-clang fails as you expected :(
And set LLVM_DEFAULT_TARGET_TRIPLE to "" will also make check-clang fail.

My use case is native target disabled in llvm, which will fail check-llvm because LLVM_DEFAULT_TARGET_TRIPLE is set to native target which is not enabled.
That could be workaround by set LLVM_DEFAULT_TARGET_TRIPLE to "" or apply this patch.
And I also want to run check-clang, it will work by default, but it fails after set LLVM_DEFAULT_TARGET_TRIPLE "".

Is it possible to make both check-clang and check-llvm pass while native target is not enabled?
One solution could be add something like native_triple_enabled in lit config, then update all tests that check default_triple to check native_triple_enabled.
Is there another way to make it work?

[smeenai]

In D134972#3847325, @python3kgae wrote:

In D134972#3847120, @smeenai wrote:

In D134972#3844420, @python3kgae wrote:

In D134972#3844340, @smeenai wrote:

Sorry for the slow review.

Does this mean that LLVM_DEFAULT_TARGET_TRIPLE will now be empty when the native target isn't enabled (since LLVM_DEFAULT_TARGET_TRIPLE_default won't be set)? Does that cause any problems?

Thanks for reviewing.
Yes. Now LLVM_DEFAULT_TARGET_TRIPLE will be empty when the native target is not enabled.
check-llvm passed with this change (It fails when the native target is not enabled without this change. I only tested Linux build on x86 though.)

What about check-clang? What does clang do in this case if you try a compile without specifying an explicit triple?

Conceptually, requiring the user to explicitly pick a default target triple from one of the backends they have enabled seems cleaner than having it be empty to me. We'd error out if you don't have the native target enabled and didn't explicitly specify LLVM_DEFAULT_TARGET_TRIPLE. Would that work for you?

check-clang fails as you expected :(
And set LLVM_DEFAULT_TARGET_TRIPLE to "" will also make check-clang fail.

My use case is native target disabled in llvm, which will fail check-llvm because LLVM_DEFAULT_TARGET_TRIPLE is set to native target which is not enabled.
That could be workaround by set LLVM_DEFAULT_TARGET_TRIPLE to "" or apply this patch.
And I also want to run check-clang, it will work by default, but it fails after set LLVM_DEFAULT_TARGET_TRIPLE "".

Is it possible to make both check-clang and check-llvm pass while native target is not enabled?
One solution could be add something like native_triple_enabled in lit config, then update all tests that check default_triple to check native_triple_enabled.
Is there another way to make it work?

How does check-clang behave if you set LLVM_DEFAULT_TARGET_TRIPLE to a triple for a backend that you do have enabled?

[python3kgae]

In D134972#3847727, @smeenai wrote:

In D134972#3847325, @python3kgae wrote:

In D134972#3847120, @smeenai wrote:

In D134972#3844420, @python3kgae wrote:

In D134972#3844340, @smeenai wrote:

Sorry for the slow review.

Does this mean that LLVM_DEFAULT_TARGET_TRIPLE will now be empty when the native target isn't enabled (since LLVM_DEFAULT_TARGET_TRIPLE_default won't be set)? Does that cause any problems?

Thanks for reviewing.
Yes. Now LLVM_DEFAULT_TARGET_TRIPLE will be empty when the native target is not enabled.
check-llvm passed with this change (It fails when the native target is not enabled without this change. I only tested Linux build on x86 though.)

What about check-clang? What does clang do in this case if you try a compile without specifying an explicit triple?

Conceptually, requiring the user to explicitly pick a default target triple from one of the backends they have enabled seems cleaner than having it be empty to me. We'd error out if you don't have the native target enabled and didn't explicitly specify LLVM_DEFAULT_TARGET_TRIPLE. Would that work for you?

check-clang fails as you expected :(
And set LLVM_DEFAULT_TARGET_TRIPLE to "" will also make check-clang fail.

My use case is native target disabled in llvm, which will fail check-llvm because LLVM_DEFAULT_TARGET_TRIPLE is set to native target which is not enabled.
That could be workaround by set LLVM_DEFAULT_TARGET_TRIPLE to "" or apply this patch.
And I also want to run check-clang, it will work by default, but it fails after set LLVM_DEFAULT_TARGET_TRIPLE "".

Is it possible to make both check-clang and check-llvm pass while native target is not enabled?
One solution could be add something like native_triple_enabled in lit config, then update all tests that check default_triple to check native_triple_enabled.
Is there another way to make it work?

How does check-clang behave if you set LLVM_DEFAULT_TARGET_TRIPLE to a triple for a backend that you do have enabled?

Both check-clang and check-llvm will fail when set LLVM_DEFAULT_TARGET_TRIPLE to the triple with target enabled. :(

[smeenai]

Hmm, I'm unable to repro that. On an x86-64 Linux machine, I set LLVM_TARGETS_TO_BUILD to just AArch64 and LLVM_DEFAULT_TARGET_TRIPLE to aarch64-linux-gnu. check-llvm and check-clang both worked for me. What values were you trying?

[python3kgae]

In D134972#3848193, @smeenai wrote:

Hmm, I'm unable to repro that. On an x86-64 Linux machine, I set LLVM_TARGETS_TO_BUILD to just AArch64 and LLVM_DEFAULT_TARGET_TRIPLE to aarch64-linux-gnu. check-llvm and check-clang both worked for me. What values were you trying?

I'm building DirectX target which is still experimental.
But I tested amdgcn-amd-amdhsa too, it also fails.

[smeenai]

Unfortunately I'm not set up for AMDGPU, so I can't test that locally. Just to confirm, check-llvm works for you if LLVM_DEFAULT_TARGET_TRIPLE is empty, but not if it's set to a triple corresponding to an enabled backend?

[python3kgae]

In D134972#3848302, @smeenai wrote:

Unfortunately I'm not set up for AMDGPU, so I can't test that locally. Just to confirm, check-llvm works for you if LLVM_DEFAULT_TARGET_TRIPLE is empty, but not if it's set to a triple corresponding to an enabled backend?

Yes. check-llvm works if LLVM_DEFAULT_TARGET_TRIPLE is empty. ( except 2 OrcV2Examples in this patch).
Then set it to amdgcn-amd-amdhsa get lot of fails.

[smeenai]

In D134972#3848313, @python3kgae wrote:

In D134972#3848302, @smeenai wrote:

Unfortunately I'm not set up for AMDGPU, so I can't test that locally. Just to confirm, check-llvm works for you if LLVM_DEFAULT_TARGET_TRIPLE is empty, but not if it's set to a triple corresponding to an enabled backend?

Yes. check-llvm works if LLVM_DEFAULT_TARGET_TRIPLE is empty. ( except 2 OrcV2Examples in this patch).
Then set it to amdgcn-amd-amdhsa get lot of fails.

Hmm. Do you have an example of a failing test and why it's failing? (I see lots of amdhsa failures on my end, but I'm pretty sure it's just because I'm not set up correctly.) I'm trying to understand why the default triple is making a difference here, vs. just having the backend enabled or not enabled, and what the behavior is when the default triple is empty that's making the test pass.

[python3kgae]

In D134972#3857363, @smeenai wrote:

In D134972#3848313, @python3kgae wrote:

In D134972#3848302, @smeenai wrote:

Unfortunately I'm not set up for AMDGPU, so I can't test that locally. Just to confirm, check-llvm works for you if LLVM_DEFAULT_TARGET_TRIPLE is empty, but not if it's set to a triple corresponding to an enabled backend?

Yes. check-llvm works if LLVM_DEFAULT_TARGET_TRIPLE is empty. ( except 2 OrcV2Examples in this patch).
Then set it to amdgcn-amd-amdhsa get lot of fails.

Hmm. Do you have an example of a failing test and why it's failing? (I see lots of amdhsa failures on my end, but I'm pretty sure it's just because I'm not set up correctly.) I'm trying to understand why the default triple is making a difference here, vs. just having the backend enabled or not enabled, and what the behavior is when the default triple is empty that's making the test pass.

For check-llvm,
amdgcn-amd-amdhsa will cause many amdgpu test failures because amdhsa not work for some tests. Change it to amdgcn-amd- could avoid all amdgpu codeGen test fails.
The rest of fails might caused by amdgcn doesn't support all the things CPU supports and fail in instruction selection.
For default triple empty, these tests are ignored as unsupported by "# REQUIRES: default_triple".

For check-clang,
Lit will report fatal: Could not turn 'amdgcn-amd-' into Itanium ABI triple. Something for empty default triple.
amdgcn-amd-amdhsa will get many tests fail for cannot find ROCm device library.

[bogner]

FWIW this seems like a sensible change to me. The testing infrastructure for handling an empty triple is all there. It's sad that the clang tests don't pass, but that's a pre-existing issue with no default triple so I don't think it should block us here.

In D134972#3847120, @smeenai wrote:

Conceptually, requiring the user to explicitly pick a default target triple from one of the backends they have enabled seems cleaner than having it be empty to me. We'd error out if you don't have the native target enabled and didn't explicitly specify LLVM_DEFAULT_TARGET_TRIPLE. Would that work for you?

I'm not necessarily opposed to some kind of warning here, but we would have to be careful not to error when the user explicitly sets an empty triple. This is fairly common when you want users of the compiler to have to choose a backend explicitly.

[smeenai]

Sorry, this fell off my radar. This seems like it solves your problem, it's clean enough, and the alternative I had in mind wasn't significantly better, so we can go ahead with this.

[python3kgae]

Rebase

[python3kgae]

In D134972#3908924, @smeenai wrote:

Sorry, this fell off my radar. This seems like it solves your problem, it's clean enough, and the alternative I had in mind wasn't significantly better, so we can go ahead with this.

Thanks for the review.
I'll see if it is possible to make check-clang work without native target next.

[sgraenitz]

@python3kgae Just randomly spotted your patch. Thanks! (Why wouldn't Herald add me to the review..?)

llvm/CMakeLists.txt

 include(config-ix)
 
 # By default, we target the host, but this can be overridden at CMake
 # invocation time. Except on 64-bit AIX, where the system toolchain
 # expect 32-bit objects by default.
 if("${LLVM_HOST_TRIPLE}" MATCHES "^powerpc64-ibm-aix")
   string(REGEX REPLACE "^powerpc64" "powerpc" LLVM_DEFAULT_TARGET_TRIPLE_default "${LLVM_HOST_TRIPLE}")
 else()
+  # Only set default triple when native target is enabled.
+  if (LLVM_NATIVE_TARGET)
     set(LLVM_DEFAULT_TARGET_TRIPLE_default "${LLVM_HOST_TRIPLE}")
   endif()
+endif()
 
 set(LLVM_DEFAULT_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE_default}" CACHE STRING
   "Default target for which LLVM will generate code." )
 if (TARGET_TRIPLE)
   message(WARNING "TARGET_TRIPLE is deprecated and will be removed in a future release. "
                   "Please use LLVM_DEFAULT_TARGET_TRIPLE instead.")
   set(LLVM_TARGET_TRIPLE "${TARGET_TRIPLE}")
 else()

llvm/test/Examples/OrcV2Examples/lljit-with-remote-debugging.test

 # This test makes sure that the example builds and executes as expected.
 # Instructions for debugging can be found in LLJITWithRemoteDebugging.cpp
 
+# REQUIRES: default_triple
+
 # RUN: LLJITWithRemoteDebugging %p/Inputs/argc_sub1_elf.ll | FileCheck --check-prefix=CHECK0 %s
 # CHECK0: Parsing input IR code from: {{.*}}/Inputs/argc_sub1_elf.ll
 # CHECK0: Running: main()
 # CHECK0: Exit code: 0
 
 # RUN: LLJITWithRemoteDebugging %p/Inputs/argc_sub1_elf.ll --args 2nd 3rd 4th | FileCheck --check-prefix=CHECK3 %s
 # CHECK3: Parsing input IR code from: {{.*}}/Inputs/argc_sub1_elf.ll
 # CHECK3: Running: main("2nd", "3rd", "4th")
 # CHECK3: Exit code: 3

llvm/test/Examples/OrcV2Examples/lljit-with-thinlto-summaries.test

 # RUN: opt -module-summary %p/Inputs/main-mod.ll -o main-mod.bc
 # RUN: opt -module-summary %p/Inputs/foo-mod.ll -o foo-mod.bc
 # RUN: opt -module-summary %p/Inputs/bar-mod.ll -o bar-mod.bc
 
+# REQUIRES: default_triple
+
 # RUN: llvm-lto -thinlto -o main-foo-bar main-mod.bc foo-mod.bc bar-mod.bc
 
 # RUN: LLJITWithThinLTOSummaries main-foo-bar.thinlto.bc 2>&1 | FileCheck %s
 
 # CHECK: About to load module: main-mod.bc
 # CHECK: About to load module: foo-mod.bc
 # CHECK: About to load module: bar-mod.bc
 # CHECK: 'main' finished with exit code: 0