[compiler-rt] Avoid assertions when using LLVM_ENABLE_PER_TARGET_RUNTIME_DIR

[ilovepi]

Previously, the memprof and ctx_profile lit.site.cfg would assert
if the enable_per_target_runtime_dir was set and the
config.target_arch != config.host_arch. However, config.host_arch would
never be set, meaning this would always fail in a when using
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.

This patch follows the example in the ASAN lit.site.cfg.py that updates
the compiler_rt_libdir appropriately.

[llvmbot]

@llvm/pr-subscribers-pgo

Author: Paul Kirth (ilovepi)

Changes

Previously, the memprof and ctx_profile lit.site.cfg would assert
if the enable_per_target_runtime_dir was set and the
config.target_arch != config.host_arch. However, config.host_arch would
never be set, meaning this would always fail in a when using
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.

This patch follows the example in the ASAN lit.site.cfg.py that updates
the compiler_rt_libdir appropriately.

---

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

2 Files Affected:

• (modified) compiler-rt/test/ctx_profile/Unit/lit.site.cfg.py.in (+8-4)
• (modified) compiler-rt/test/memprof/Unit/lit.site.cfg.py.in (+8-5)

diff --git a/compiler-rt/test/ctx_profile/Unit/lit.site.cfg.py.in b/compiler-rt/test/ctx_profile/Unit/lit.site.cfg.py.in
index 32a8c48e9c1c79..bf586bfab55237 100644
--- a/compiler-rt/test/ctx_profile/Unit/lit.site.cfg.py.in
+++ b/compiler-rt/test/ctx_profile/Unit/lit.site.cfg.py.in
@@ -21,7 +21,11 @@ config.test_source_root = config.test_exec_root
 # When LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on, the initial value of
 # config.compiler_rt_libdir (COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR) has the
 # host triple as the trailing path component. The value is incorrect for i386
-# tests on x86_64 hosts and vice versa. But, since only x86_64 is enabled as
-# target, and we don't support different environments for building and,
-# respectivelly, running tests, we shouldn't see this case.
-assert not config.enable_per_target_runtime_dir or config.target_arch == config.host_arch
+# tests on x86_64 hosts and vice versa. Adjust config.compiler_rt_libdir
+# accordingly.
+if config.enable_per_target_runtime_dir and config.target_arch != config.host_arch:
+    if config.target_arch == 'i386':
+        config.compiler_rt_libdir = re.sub(r'/x86_64(?=-[^/]+$)', '/i386', config.compiler_rt_libdir)
+    elif config.target_arch == 'x86_64':
+        config.compiler_rt_libdir = re.sub(r'/i386(?=-[^/]+$)', '/x86_64', config.compiler_rt_libdir)
+
diff --git a/compiler-rt/test/memprof/Unit/lit.site.cfg.py.in b/compiler-rt/test/memprof/Unit/lit.site.cfg.py.in
index c968e403a44d09..b9485024fa5082 100644
--- a/compiler-rt/test/memprof/Unit/lit.site.cfg.py.in
+++ b/compiler-rt/test/memprof/Unit/lit.site.cfg.py.in
@@ -21,10 +21,13 @@ config.test_source_root = config.test_exec_root
 # When LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on, the initial value of
 # config.compiler_rt_libdir (COMPILER_RT_RESOLVED_LIBRARY_OUTPUT_DIR) has the
 # host triple as the trailing path component. The value is incorrect for i386
-# tests on x86_64 hosts and vice versa.  But, since only x86_64 is enabled as
-# target, and we don't support different environments for building and,
-# respectivelly, running tests, we shouldn't see this case.
-assert not config.enable_per_target_runtime_dir or config.target_arch == config.host_arch
+# tests on x86_64 hosts and vice versa. Adjust config.compiler_rt_libdir
+# accordingly.
+if config.enable_per_target_runtime_dir and config.target_arch != config.host_arch:
+    if config.target_arch == 'i386':
+        config.compiler_rt_libdir = re.sub(r'/x86_64(?=-[^/]+$)', '/i386', config.compiler_rt_libdir)
+    elif config.target_arch == 'x86_64':
+        config.compiler_rt_libdir = re.sub(r'/i386(?=-[^/]+$)', '/x86_64', config.compiler_rt_libdir)
 
 if not config.parallelism_group:
-  config.parallelism_group = 'shadow-memory'
\ No newline at end of file
+  config.parallelism_group = 'shadow-memory'

[mtrofin]

I would prefer not having the general handling of asan, because we don't mean to be here for 32 bit anyway. so if config.host_arch is not set in your case, what's the rest - I assume config.target_arch is exactly x86_64 and then we could assert that and do the behavior on line 30?

[ilovepi]

I would prefer not having the general handling of asan, because we don't mean to be here for 32 bit anyway.

Fair.

so if config.host_arch is not set in your case, what's the rest - I assume config.target_arch is exactly x86_64 and then we could assert that and do the behavior on line 30?

The config.target_arch is set as expected. There is already an assert that its X86_64 on line 14. So maybe just

if config.enable_per_target_runtime_dir:
  ...

[ilovepi]

ah, but that's only for the CTX_PROFILE.

Maybe a better approach is to assert it isn't i386? Then you have the ASAN like fixup on the libdir and 32-bit is still unsupported

[mtrofin]

sgtm. we have an assert earlier. OK.

[ilovepi]

Since we have the assert in both ctx_profile and memprof, I think this simplified check is appropriate.

[mtrofin]

lgtm for ctx_profile