[flang][Driver] Guard check for pic/pie settings without driver flags

[kparzysz]

The default relocation model for clang depends on the cmake flag CLANG_DEFAULT_PIE_ON_LINUX. By default it is set to ON, but when it's OFF, the default relocation model will be "static".
The outcome of the test running clang without any PIC/PIE flags will depend on the cmake flag, so make sure it only runs when the flag is ON.

[llvmbot]

@llvm/pr-subscribers-flang-driver

Author: Krzysztof Parzyszek (kparzysz)

Changes

The default relocation model for aarch64 seems to be "static", but the pic-flags.f90 test was expecting both pic and pie settings.

---

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

1 Files Affected:

• (modified) flang/test/Driver/pic-flags.f90 (-1)

diff --git a/flang/test/Driver/pic-flags.f90 b/flang/test/Driver/pic-flags.f90
index cb62d353cc18c..d63a69c913fa1 100644
--- a/flang/test/Driver/pic-flags.f90
+++ b/flang/test/Driver/pic-flags.f90
@@ -1,6 +1,5 @@
 ! RUN: %if aarch64-registered-target %{ %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fno-pie 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-STATIC,CHECK-STATIC-IR %}
 
-! RUN: %if aarch64-registered-target %{ %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-PIE-LEVEL2,CHECK-PIE-LEVEL2-IR %}
 ! RUN: %if aarch64-registered-target %{ %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fpie 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-PIE-LEVEL1,CHECK-PIE-LEVEL1-IR %}
 ! RUN: %if aarch64-registered-target %{ %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fPIE 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-PIE-LEVEL2,CHECK-PIE-LEVEL2-IR %}
 

[kparzysz]

This test always fails with my local build. I have all targets enabled (with x86_64 being the default one), but maybe it needs some specific build flags for the default behavior to be as expected by the test.

It's only the part that checks for PIC/PIE without specifying any command-line options that fails, the invocations that specify PIC/PIE work fine.

[pawosm-arm]

Does not fail on us. Maybe because we use -DLLVM_ENABLE_PIC=ON CMake flag? https://github.com/arm/arm-toolchain/blob/ade702ad094b1fec1aaa784eb269a5b01c8eda7c/arm-software/linux/build.sh#L60C5-L60C25

[kparzysz]

I think LLVM_ENABLE_PIC controls whether PIC is used to compile LLVM itself.

Here are my cmake flags:
cmake -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_LIT_ARGS='-sv -j64' -DCMAKE_BUILD_TYPE=Release -DCLANG_DEFAULT_LINKER=lld -DLLVM_BUILD_TESTS=ON -DLLVM_INCLUDE_TESTS=ON -DCLANG_INCLUDE_TESTS=ON -DLLVM_TARGETS_TO_BUILD=all -DLLVM_ENABLE_RUNTIMES='openmp;offload;compiler-rt;libcxx;libcxxabi;libunwind' -DLLVM_ENABLE_PROJECTS='bolt;clang;clang-tools-extra;flang;lld;lldb;mlir;polly;pstl' -DBUILD_SHARED_LIBS=ON -DLIBOMPTARGET_PLUGINS_TO_BUILD='amdgpu;host' -DCLANG_DEFAULT_PIE_ON_LINUX=OFF -DLIBOMP_OMPT_SUPPORT=OFF -DLLVM_OMIT_DAGISEL_COMMENTS=OFF

Maybe it's the CLANG_DEFAULT_PIE_ON_LINUX=OFF? In any case, a check like that in a testcase should probably be guarded by something.

[pawosm-arm]

Maybe it's the CLANG_DEFAULT_PIE_ON_LINUX=OFF? In any case, a check like that in a testcase should probably be guarded by something.

No idea, could be, we're not setting it.

[kparzysz]

I removed the CLANG_DEFAULT_PIE_ON_LINUX=OFF flag, and the test now passes.

I still think it should be removed because it depends on build options, and can fail even when everything works as intended.

[pawosm-arm]

Let Andrzej (@banach-space) decide, he created this file.

[banach-space]

Hey @kparzysz , thanks for digging into this!

I still think it should be removed because it depends on build options, and can fail even when everything works as intended.

How about adding an extra condition to that RUN line:

! RUN: %if aarch64-registered-target && clang_default_pie_on_linux %{ %flang ...}

? clang_default_pie_on_linux should already be available in LIT: https://github.com/llvm/llvm-project/blob/main/clang/test/lit.site.cfg.py.in#L26

[banach-space]

LGTM, thanks!

[nit] Could you document somewhere (commit summary?) what makes clang_default_pie_on_linux necessary in this particular case?