Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues with using a non-default system LLVM with CHPL_LLVM_CONFIG #26428

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 51 additions & 8 deletions util/chplenv/chpl_llvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,42 @@ def get_clang_prgenv_args():

return (comp_args, link_args)

@memoize
def get_system_include_directories(flag, lang):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this code will only work for gcc and clang. I'd recommend limiting it to those compilers. We can add other compilers as needed.

"""
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_system_include_directory(path, flag, lang):
"""
Returns True if the given path is a system include directory
"""
directories = get_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])

# 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 @@ -1048,18 +1084,25 @@ 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
# 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)
#
# when adding LLVM=bundled, we should include the LLVM headers as system
# headers and prefer the bundled headers, so use -isystem
#
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":
if not is_path_system_include_directory(directory, 'host', 'c++'):
# its not included by the compiler, so its safe to use -isystem
ret.append('-isystem' + directory)
else:
# technically don't need to add this for
# but it's harmless to use -I
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment wording got weird

ret.append("-I" + directory)
else:
# bundled LLVM is always safe to use -isystem
ret.append("-isystem" + directory)
continue

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