Skip to content

Commit

Permalink
Fix issues with using a non-default system LLVM with CHPL_LLVM_CONFIG (
Browse files Browse the repository at this point in the history
…#26428)

Fixes compiler build issues when using a system LLVM that is different
than the standard LLVM. For example, an LLVM installed at `/usr/` would
conflict with an LLVM installed at `/home/user/llvm-install` (which was
accessed by `CHPL_LLVM_CONFIG=/home/user/llvm-install`)

Subsumes #26212 and
#26402

Previous work had tried to take advantage of various clang/gcc flags to
achieve the right behavior, however this not working in all cases. This
PR does following.

1. computes the existing search paths for the compiler
2. for each LLVM include directory specified as `-I`
  - In a bundled LLVM build: always use '-isystem'
- In a system LLVM build: use '-isystem' if its not an existing search
path, otherwise use '-I'

Testing
- [x] `make check` with HOST_CC=clang, LLVM=system
- [x] `make check` with HOST_CC=clang, LLVM=bundled
- [x] `make check` with HOST_CC=clang, LLVM=system,
LLVM_CONFIG=/some/path, with/without a normal system LLVM
- [x] `make check` with HOST_CC=gnu, LLVM=system
- [x] `make check` with HOST_CC=gnu, LLVM=bundled
- [x] `make check` with HOST_CC=gnu, LLVM=system,
LLVM_CONFIG=/some/path, with/without a normal system LLVM
- [x] Sanity check that normal compilations on Mac are not broken

[Reviewed by @mppf]
  • Loading branch information
jabraham17 authored Jan 6, 2025
2 parents 8d2f1e4 + 7b6afca commit 9b9894a
Showing 1 changed file with 69 additions and 8 deletions.
77 changes: 69 additions & 8 deletions util/chplenv/chpl_llvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,52 @@ def get_clang_prgenv_args():

return (comp_args, link_args)

@memoize
def get_clang_gcc_system_include_directories(flag, lang):
"""
Runs 'COMPILER -E -Wp,-v -' to determine the system include directories
"""
directories = []
compiler = chpl_compiler.get_compiler_command(flag, lang)
_, _, stdout, _ = try_run_command(
compiler + ["-E", "-Wp,-v", "-", "-o", "/dev/null"],
combine_output=True, cmd_input=""
)
if stdout:
lines = stdout.splitlines()
start_includes_regex = re.compile(r"#include.+search starts here")
end_includes_regex = re.compile(r"End of search list")
ignoring_regex = re.compile(r"(?:ignoring nonexistent)|(?:as it is a non-system)")
collecting = False
for line in lines:
if start_includes_regex.search(line):
collecting = True
elif end_includes_regex.search(line):
collecting = False
break
elif collecting and not ignoring_regex.search(line):
directories.append(line.strip())
return directories

@memoize
def is_path_clang_gcc_system_include_directory(path, flag, lang):
"""
Returns True if the given path is a system include directory
"""
directories = get_clang_gcc_system_include_directories(flag, lang)
p = os.path.realpath(os.path.abspath(path))
return any([os.path.realpath(os.path.abspath(d)) == p for d in directories])

@memoize
def can_infer_system_include_directories(flag, lang):
"""
Returns True if the system include directories can be inferred
"""
compiler = chpl_compiler.get_compiler_command(flag, lang)
inferred = chpl_compiler.get_compiler_from_command(compiler[0])
return inferred in ('clang', 'gnu')


# Filters out C++ compilation flags from llvm-config.
# The flags are passed as a list of strings.
# Returns a list of strings containing the kept flags.
Expand All @@ -1112,18 +1158,33 @@ def filter_llvm_config_flags(llvm_val, flags):
continue # filter out these flags

#
# include LLVM headers as system headers
# include LLVM headers as system headers using -isystem
# this avoids warnings inside of LLVM headers by treating LLVM headers
#
# when adding LLVM=system as system headers, we should not perturb the
# include search path, so use -isystem-after/-idirafter
#
# when adding LLVM=bundled, we should include the LLVM headers as system
# headers and prefer the bundled headers, so use -isystem
# If the header is already a system header, using -isystem will break
# the include search. In that case, just use -I (which technically wont do anythings)
#
include_flag = '-idirafter' if llvm_val == 'system' else '-isystem'
if flag.startswith('-I'):
ret.append(include_flag + flag[2:])
directory = flag[2:]
if llvm_val == "system":
can_infer = can_infer_system_include_directories("host", "c++")
is_system_inc = is_path_clang_gcc_system_include_directory(
directory, "host", "c++"
)
if can_infer and not is_system_inc:
# the directory not included by the compiler,
# so its safe to use -isystem
ret.append("-isystem" + directory)
else:
# technically don't need to explicitly add the directory to
# the include path if it's a system header. but it is
# harmless to use just '-I'
# if we can't determine the system include directories,
# we have to add it anyways
ret.append("-I" + directory)
else:
# for the bundled LLVM it is always safe to use -isystem
ret.append("-isystem" + directory)
continue

if flag.startswith('-W'):
Expand Down

0 comments on commit 9b9894a

Please sign in to comment.