# Projects

User does not belong to any projects.

# User Details

User Since
Mar 4 2015, 5:44 PM (289 w, 4 d)

# Fri, Sep 18

phosek added a comment to D87928: Provide -fsource-dir flag in Clang.

This change is trying to address the issues raised in D83154. There are still some open questions:

• Is -fsource-dir the best name for this flag?
• I'm not sure if make_relative should be applied to all source paths, or only paths that start with SourceDir which would exclude system paths outside of the source directory (e.g. it's probably undesirable to relativize paths to /usr/include)?
• If we decide to exclude source paths outside of the source directory, should we support -fsource-dir to be specified more then once to handle multiple source directories?
Fri, Sep 18, 12:23 PM · Restricted Project, Restricted Project
In D83154#2277539, @rnk wrote:

Yeah, my goal is for the build system to be able to avoid having to embed PWD into the compiler flags. For example, I believe it is a goal of the LLVM gn build system for the build tree to be relocatable. If the compiler command lines need to contain absolute paths to make the paths in the coverage relative, we won't be able to achieve that goal.

Based on all the comments that have gone before, it sounds like we want a -fprofile-compilation-dir= flag. Once we have that, would this logic work?

• if profile compilation dir set, absolutize with that as the root
• if no profile compilation dir, absolutize with CWD
• apply profile prefix map

Should -no-canonical-prefixes also be involved here? Unclear.

I originally considered -fprofile-compilation-dir= and perhaps -ffile-compilation-dir= to also handle cases like __FILE__ macro (-ffile-prefix-map and -fmacro-prefix-map suffer from the same issue as -fprofile-prefix-map), but I'm not sure if specifying compilation dir is the right approach. It makes sense for debug info because -fdebug-compilation-dir is used directly as DW_AT_comp_dir and also because it's a convention for source file paths in debug info to be relative to build directory, but it's not the case for paths in coverage mapping or __FILE__.

So what I'm considering now and started prototyping is a -fsource-dirflag (or maybe -fsource-root-dir, I'm open to suggestions for the name) where the semantics would be that if this flag is set, all source paths will be made relative to this directory. There's an open question whether this should also apply to sources outside of that directory, for example system header paths.

The example use case, assuming source is in /path/to/source and build is in /path/to/build, would be: clang -fdebug-compilation-dir . -fsource-dir ../source ../source/dir/file.c. In this case __FILE__ would expand to dir/file.c and that path would be also stored in coverage mapping.

Fri, Sep 18, 12:17 PM · Restricted Project
phosek added reviewers for D87928: Provide -fsource-dir flag in Clang: .
Fri, Sep 18, 12:17 PM · Restricted Project, Restricted Project
phosek requested review of D87928: Provide -fsource-dir flag in Clang.
Fri, Sep 18, 12:17 PM · Restricted Project, Restricted Project

# Wed, Sep 16

phosek added a comment to D87732: [Support] Provide sys::path::guess_style.

I'm not convinced this is really correct. After all, a path with mixed separators (e.g. '/my/path\to\foo') could be a Windows path rooted at the current drive. This mixed style can sometimes happen when things get concatenated together, so I don't think it's compeletely unreasonable.

I'm also concerned that if we guess wrong, the behaviour will end up breaking. For example, if the path were a Windows path "/my/path\../to/foo", I'd expect a remove_dots call (without worrying about slash normalisation) to result in "/my/to/foo", but if the code is using Posix style, will it actually result in the path being left unchanged?

I can think of two competing alternatives:

1. add an option to remove_dots to not do the separator canonicalization. I'm not sure whether this can be done unambiguously however - which separator should be removed when a directory is removed due to dots? The one before the removed parts? After them?
2. change all '\' to '/' unconditionally. However, this might break Linux paths with '\' in.
Wed, Sep 16, 5:02 PM · Restricted Project
Revert "[lsan] Share platform allocator settings between ASan and LSan"
Wed, Sep 16, 1:49 PM · Restricted Project
In D83154#2277539, @rnk wrote:

Yeah, my goal is for the build system to be able to avoid having to embed PWD into the compiler flags. For example, I believe it is a goal of the LLVM gn build system for the build tree to be relocatable. If the compiler command lines need to contain absolute paths to make the paths in the coverage relative, we won't be able to achieve that goal.

Based on all the comments that have gone before, it sounds like we want a -fprofile-compilation-dir= flag. Once we have that, would this logic work?

• if profile compilation dir set, absolutize with that as the root
• if no profile compilation dir, absolutize with CWD
• apply profile prefix map

Should -no-canonical-prefixes also be involved here? Unclear.

Wed, Sep 16, 1:40 PM · Restricted Project
phosek committed rGc57df3dc09e8: [lsan] Share platform allocator settings between ASan and LSan (authored by phosek).
[lsan] Share platform allocator settings between ASan and LSan
Wed, Sep 16, 1:31 PM · Restricted Project

# Tue, Sep 15

phosek updated the diff for D87732: [Support] Provide sys::path::guess_style.
Tue, Sep 15, 11:47 PM · Restricted Project
phosek added a comment to D87732: [Support] Provide sys::path::guess_style.

Hmm - probably simpler to justify this patch by including refactoring at least one (possibly more) existing uses you mentioned, to use this API - then I probably don't really need to understand what it's doing, just that it's generalizing some existing functionality.

Tue, Sep 15, 11:46 PM · Restricted Project
phosek updated the diff for D87732: [Support] Provide sys::path::guess_style.
Tue, Sep 15, 11:33 PM · Restricted Project
phosek updated the diff for D87732: [Support] Provide sys::path::guess_style.
Tue, Sep 15, 11:31 PM · Restricted Project
Tue, Sep 15, 5:11 PM · Restricted Project
phosek added a comment to D87732: [Support] Provide sys::path::guess_style.

I had to revert D87657 because it broke several tests: the normalization resulted in slashes being converted to native format, but that's incorrect. I'd like to use this function to instead determine the style from the path inside the debug info.

Tue, Sep 15, 5:10 PM · Restricted Project
phosek requested review of D87732: [Support] Provide sys::path::guess_style.
Tue, Sep 15, 5:08 PM · Restricted Project
Tue, Sep 15, 11:42 AM · Restricted Project
Revert "[DebugInfo] Remove dots from getFilenameByIndex return value"
Tue, Sep 15, 10:08 AM · Restricted Project
phosek updated the diff for D87656: [llvm-dwarfdump] --show-sources option to show all sources.
Tue, Sep 15, 2:04 AM · Restricted Project
phosek committed rG58938b544b72: [NFC][DebugInfo] Use consistent regex group spelling (authored by phosek).
[NFC][DebugInfo] Use consistent regex group spelling
phosek committed rGc1f2fb5184ca: [DebugInfo] Support both forward and backward slashes in tests (authored by phosek).
[DebugInfo] Support both forward and backward slashes in tests

# Mon, Sep 14

phosek committed rG042c23506869: [DebugInfo] Remove dots from getFilenameByIndex return value (authored by phosek).
[DebugInfo] Remove dots from getFilenameByIndex return value
Mon, Sep 14, 8:31 PM · Restricted Project
phosek added a comment to D87657: [DebugInfo] Remove dots from getFilenameByIndex return value.

So a trivial example is:

#include <string>
int main() {}

When compiled as:

clang++ test.cc -g3 -S -o test.S

I see paths like these in the output:

"/usr/local/google/home/phosek/fuchsia" "prebuilt/third_party/clang/linux-x64/bin/../include/c++/v1/cstddef"

We could go over driver code and make sure we always invoke sys::path::remove_dots inside methods like AddIncludePaths or addPathIfExists (see for example https://github.com/llvm/llvm-project/blob/b3afad046301d8bb1f4471aceaad704b87de3a69/clang/lib/Driver/ToolChains/Gnu.cpp#L2902 which where the .. in libc++ header path comes from), but maybe it'd be better to do the normalization uniformly when emitting debuginfo metadata?

Mon, Sep 14, 7:14 PM · Restricted Project
phosek added a comment to D87657: [DebugInfo] Remove dots from getFilenameByIndex return value.

Got any particular examples of this from LLVM's DWARF output? I suspect this sort of thing might come from/lead to path non-canonicalization which would make the line table bigger than it needs to be, so might be worth fixing there too.

Mon, Sep 14, 6:11 PM · Restricted Project
phosek requested review of D87657: [DebugInfo] Remove dots from getFilenameByIndex return value.
Mon, Sep 14, 5:51 PM · Restricted Project
phosek added a comment to D87656: [llvm-dwarfdump] --show-sources option to show all sources.

While this information can be extracted out of the existing llvm-dwarfdump output, it requires additional post-processing. This became so common in our project that we have implemented a custom Go-based tool for that purpose, but that has other downsides such as the lack of DWARF5 support. I think that supporting this option directly in llvm-dwarfdump might be generally useful and it doesn't add a lot of complexity. I'm open to suggestions for how to improve the output.

Mon, Sep 14, 5:40 PM · Restricted Project
phosek requested review of D87656: [llvm-dwarfdump] --show-sources option to show all sources.
Mon, Sep 14, 5:36 PM · Restricted Project

# Fri, Sep 11

LGTM

Fri, Sep 11, 11:01 AM · Restricted Project

# Wed, Sep 9

phosek committed rGf7941d980918: [lit] Use correct variable name for libxml2 (authored by phosek).
[lit] Use correct variable name for libxml2
phosek committed rGc4d7536136b3: [CMake] Simplify CMake handling for libxml2 (authored by phosek).
[CMake] Simplify CMake handling for libxml2
Wed, Sep 9, 9:45 PM · Restricted Project
phosek updated the diff for D84563: [CMake] Simplify CMake handling for libxml2.
Wed, Sep 9, 9:43 PM · Restricted Project
phosek updated the diff for D84563: [CMake] Simplify CMake handling for libxml2.
Wed, Sep 9, 5:59 PM · Restricted Project

# Tue, Sep 8

It's not clear why couldn't we support per-target runtime directory? It uses the standard multiarch layout, so in you'd end up using a directory like [path/to/resource-dir]/lib/armv6m-unknown-eabi/libclang_rt.builtins.a. I'd find this more preferable for consistency with other targets, it'd also enable the use of runtimes build for baremetal.

Tue, Sep 8, 7:46 PM · Restricted Project

# Mon, Sep 7

LGTM

Mon, Sep 7, 12:16 AM · Restricted Project

LGTM

Mon, Sep 7, 12:15 AM · Restricted Project

LGTM

Mon, Sep 7, 12:15 AM · Restricted Project

LGTM

Mon, Sep 7, 12:14 AM · Restricted Project

# Wed, Sep 2

This broke our build with the following error:

FAILED: libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_exception_storage.cpp.obj
/b/s/w/ir/k/staging/llvm_build/./bin/clang++ --target=aarch64-unknown-fuchsia --sysroot=/b/s/w/ir/k/cipd/sdk/arch/arm64/sysroot  -DHAVE___CXA_THREAD_ATEXIT_IMPL -DLIBCXXABI_USE_LLVM_UNWINDER -D_DEBUG -D_LIBCPP_DISABLE_EXTERN_TEMPLATE -D_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS -D_LIBCXXABI_BUILDING_LIBRARY -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/b/s/w/ir/k/llvm-project/libcxxabi/include -I/b/s/w/ir/k/llvm-project/libunwind/include -I/b/s/w/ir/k/llvm-project/libcxxabi/../libcxx/include --target=aarch64-unknown-fuchsia -I/b/s/w/ir/k/cipd/sdk/pkg/fdio/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -ffunction-sections -fdata-sections -ffile-prefix-map=/b/s/w/ir/k/staging/llvm_build/runtimes/runtimes-aarch64-unknown-fuchsia-bins=../staging/llvm_build/runtimes/runtimes-aarch64-unknown-fuchsia-bins -ffile-prefix-map=/b/s/w/ir/k/llvm-project/= -no-canonical-prefixes  -O2 -g  -fPIC   -nostdinc++ -Werror=return-type -W -Wall -Wchar-subscripts -Wconversion -Wmismatched-tags -Wmissing-braces -Wnewline-eof -Wunused-function -Wshadow -Wshorten-64-to-32 -Wsign-compare -Wsign-conversion -Wstrict-aliasing=2 -Wstrict-overflow=4 -Wunused-parameter -Wunused-variable -Wwrite-strings -Wundef -Wno-suggest-override -Wno-error -pedantic -fstrict-aliasing -funwind-tables -D_DEBUG -UNDEBUG -std=c++17 -MD -MT libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_exception_storage.cpp.obj -MF libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_exception_storage.cpp.obj.d -o libcxxabi/src/CMakeFiles/cxxabi_shared.dir/cxa_exception_storage.cpp.obj -c /b/s/w/ir/k/llvm-project/libcxxabi/src/cxa_exception_storage.cpp
In file included from /b/s/w/ir/k/llvm-project/libcxxabi/src/cxa_exception_storage.cpp:15:
In file included from /b/s/w/ir/k/llvm-project/libcxxabi/../libcxx/include/chrono:827:
#include <sys/cdefs.h>
^~~~~~~~~~~~~
1 error generated.

# Tue, Sep 1

LGTM

Tue, Sep 1, 11:06 PM · Restricted Project

LGTM

Tue, Sep 1, 12:55 AM · Restricted Project

# Mon, Aug 31

phosek committed rG3c7bfbd6831b: [CMake] Use find_library for ncurses (authored by phosek).
[CMake] Use find_library for ncurses
Mon, Aug 31, 8:06 PM · Restricted Project, Restricted Project, Restricted Project

LGTM

Mon, Aug 31, 1:12 PM · Restricted Project, Restricted Project, Restricted Project

This was discussed when LLVM_ENABLE_PER_TARGET_RUNTIME_DIR was introduced and there was a pushback against changing the driver behavior depending on the value of that option, so if we're going to reverse that decision for BareMetal, I think that deserves a broader discussion.

Mon, Aug 31, 1:10 PM · Restricted Project

# Fri, Aug 28

LGTM

Fri, Aug 28, 4:12 PM · Restricted Project

# Thu, Aug 27

phosek added a comment to D77248: [llvm][IR] Add a new Constant (UnnamedFunc).

This is just a naming suggestion, but how about naming it unnamed_func (instead of unnamedfunc) so it's closer to unnamed_addr.

Thu, Aug 27, 4:49 PM · Restricted Project
phosek added a comment to D86521: Revert "Use find_library for ncurses".

I have suggested an alternative in https://reviews.llvm.org/D85820#2237357 to try and fix this forward and I was waiting for @haampie's response before landing this. I can implement that today if you're OK fixing this forward? I think it'd be easier to do that than reverting and then relanding all these changes again. @gkistanova how did you check the patch with http://lab.llvm.org:8011/builders/lld-perf-testsuite bot? Is there a way to do a presubmit check on that bot?

Thu, Aug 27, 1:20 PM · Restricted Project, Restricted Project, Restricted Project

@ldionne this is still failing and it's been several days now, can we revert this change?

Sorry folks, I was gone on vacation. I see you've reverted the change -- that's fine. I'll check it in again with a workaround for older CMakes.

In this case, the workaround will have to be that the libc++ benchmarks are *not* available on older CMakes. The issue is actually deeper than that, it is that the LLVM CMakeLists.txt is setting the CMAKE_CXX_STANDARD to 14 globally: https://github.com/llvm/llvm-project/blob/1c060aa988452508fcfc65357f4a6467f3f88cc1/llvm/CMakeLists.txt#L53.
There's no easy way to work around this AFAICT beyond using proper CMake target features. Note that we should stop setting the CMAKE_CXX_STANDARD (or any global flag really) in LLVM's CMakeLists.txt, but that's another story.

phosek added a comment to D85176: [Coverage] Enable emitting gap area between macros.

We started seeing assertion failure after rolling a toolchain that contains this change and git bisect identified this change. I have filed a bug with a reproducer as PR47324.

Thu, Aug 27, 1:14 AM · Restricted Project

# Tue, Aug 25

phosek added a comment to D85820: Use find_library for ncurses.

@gkistanova It's true that this change has lead to more issues I could ever imagine, but I think the link you provided is the last remaining problem.

Pinging @phosek for a similar issue w.r.t. zlib: since https://reviews.llvm.org/D79219 zlib gets disabled on static builds of LLVM. The reason is find_package(ZLIB) finds a shared library, and check_symbol_exists(compress2 zlib.h HAVE_ZLIB) tries to statically link to that -- it can't so zlib is disabled. I guess that's a regression too?

What would be the best way forward @phosek? A quick fix for ncurses is to add a simple check_symbol_exists test too, but that would in practice just disable ncurses in static builds. And additionally we could find static libs by adding explicit names like libtinfo.a to find_library when LLVM_BUILD_STATIC=ON. This trick won't help for zlib though, the find_library stuff is hidden inside find_package, and there is no way to target static libs it seems.

Tue, Aug 25, 2:52 PM · Restricted Project, Restricted Project, Restricted Project

# Mon, Aug 24

phosek committed rG2b3807d822c5: [CMake] Fix ncurses/zlib in LLVM_SYSTEM_LIBS for Windows GNU (authored by phosek).
[CMake] Fix ncurses/zlib in LLVM_SYSTEM_LIBS for Windows GNU
Mon, Aug 24, 11:06 PM · Restricted Project

# Sat, Aug 22

phosek added a comment to D79219: [CMake] Simplify CMake handling for zlib.

@phosek in MSYS2 (targeting x86_64-w64-windows-gnu) Zlib works properly for LLVM 10 but with master I'm now seeing:

-- Constructing LLVMBuild project information
-- DEBUG zlib_library=D:/msys64/mingw64/lib/libz.dll.a
CMake Error at lib/Support/CMakeLists.txt:9 (string):
string sub-command REGEX, mode REPLACE: regex "^(lib|)" matched an empty
string.
Call Stack (most recent call first):
lib/Support/CMakeLists.txt:226 (get_system_libname)

-- DEBUG zlib_library=D:/msys64/mingw64/lib/libz.dll.a was printed by my change to help debugging it.
FYI zlib_library is set here: https://github.com/llvm/llvm-project/blob/8e06bf6b3a2e8d25e56cd52dca0cf3ff1b37b5d1/llvm/lib/Support/CMakeLists.txt#L218

Sat, Aug 22, 7:23 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project

# Aug 21 2020

phosek updated subscribers of rG31e5f7120bdd: [CMake] Simplify CMake handling for zlib.

@phosek do you know what is required to backport this as well as the ncurses detection to the LLVM 11 release branch?

LGTM

Aug 21 2020, 1:33 PM · Restricted Project

LGTM

Aug 21 2020, 1:32 PM · Restricted Project, Restricted Project

LGTM

Aug 21 2020, 1:29 PM · Restricted Project

# Aug 20 2020

Aug 20 2020, 10:37 PM · Restricted Project

LGTM

Aug 20 2020, 5:25 PM · Restricted Project
phosek added a comment to D86313: [runtimes] Allow LLVM_BUILTIN_TARGETS to include Darwin.

I agree that the current situation is not optimal, I was never happy with the default case which is confusing for a lot of people I talked to. I would like to come up with a better solution, but I'm not sure if special casing the x86_64-apple-darwin is sufficient though. With Apple Silicon, I assume we're going to be building runtimes as universal binaries that target multiple architectures.

We could consider completely decoupling runtimes from targets. Originally, each runtime build was identified only by its target, so you'd use RUNTIMES_${target}_${variable} to set target-specific variables. That became insufficient when we introduced the support for multilib, so now you also have a name and you can use RUNTIMES_${target}+${name}_${variable}. We could switch to RUNTIMES_${name}_${variable} and it'd be the responsibility of the user to set the target explicitly if they want it (FWIW it's something we already have to do in our build anyway because of Windows where the CMake behavior when you set CMAKE_${LANG}_COMPILER_TARGET breaks the build). If not, they get the default behavior, matching the default case today. What do you think?

To clarify, what would be setting \${name} in this scenario?

Aug 20 2020, 3:29 PM · Restricted Project
phosek added a comment to D86313: [runtimes] Allow LLVM_BUILTIN_TARGETS to include Darwin.

I agree that the current situation is not optimal, I was never happy with the default case which is confusing for a lot of people I talked to. I would like to come up with a better solution, but I'm not sure if special casing the x86_64-apple-darwin is sufficient though. With Apple Silicon, I assume we're going to be building runtimes as universal binaries that target multiple architectures.

Aug 20 2020, 1:39 PM · Restricted Project

I'm pretty sure add_asm_sources() has nothing to do. The ASM language is enabled by compiler-rt anyway and CMake can recognize the files as assembly anyway.

Aug 20 2020, 1:06 PM · Restricted Project, Restricted Project, Restricted Project
phosek committed rGd58fd4e52197: [compiler-rt] Compile assembly files as ASM not C (authored by phosek).
[compiler-rt] Compile assembly files as ASM not C
Aug 20 2020, 12:35 AM · Restricted Project, Restricted Project

# Aug 19 2020

@ldionne we've seen failures in various third party libraries like BoringSSL or ICU that were caused by this change. The problem is that timespec_get and aligned_alloc are only available if __DARWIN_C_LEVEL >= __DARWIN_C_FULL, but many existing libraries define _XOPEN_SOURCE in which case __DARWIN_C_LEVEL == _POSIX_C_SOURCE unless _DARWIN_C_SOURCE or _NONSTD_SOURCE are defined as well.

[CMake] Fix an issue where get_system_libname creates an empty regex capture on…
Aug 19 2020, 2:34 PM · Restricted Project

LGTM

Aug 19 2020, 2:32 PM · Restricted Project

@haampie I landed your change but I'm seeing the following failure on sanitizer Windows bot:

CMake Error at lib/Support/CMakeLists.txt:223 (get_system_libname):
get_system_libname Function invoked with incorrect arguments for function
named: get_system_libname

The failing build is http://lab.llvm.org:8011/builders/sanitizer-windows/builds/68331

I'm going to revert the change, but I'm happy to reland it once we figure out what the problem is.

I also think this broke the Windows builds for Chrome: https://crbug.com/1119478.
CMake Error at lib/Support/CMakeLists.txt:9 (STRING):

STRING sub-command REGEX, mode REPLACE: regex "^()" matched an empty
string.

Call Stack (most recent call first):

lib/Support/CMakeLists.txt:218 (get_system_libname)

Might need zlib to repro.

Aug 19 2020, 1:19 PM · Restricted Project
phosek committed rG76bf26236f6f: [CMake] Always mark terminfo as unavailable on Windows (authored by phosek).
[CMake] Always mark terminfo as unavailable on Windows
Aug 19 2020, 11:52 AM · Restricted Project
phosek requested review of D86234: [CMake] Always mark terminfo as unavailable on Windows.
Aug 19 2020, 11:08 AM · Restricted Project

@haampie I landed your change but I'm seeing the following failure on sanitizer Windows bot:

Aug 19 2020, 11:01 AM · Restricted Project
[CMake] Fix OCaml build failure because of absolute path in system libs
Aug 19 2020, 10:38 AM · Restricted Project
[CMake] Don't look for terminfo libs when LLVM_ENABLE_TERMINFO=OFF
Aug 19 2020, 10:32 AM · Restricted Project, Restricted Project

# Aug 18 2020

This seems to have broken our Mac builders with the following error:

-- Testing: 25226 tests, 24 workers --
Testing:  0.. 10.. 20.
FAIL: Clang :: Frontend/ast-main.c (6834 of 25226)
******************** TEST 'Clang :: Frontend/ast-main.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   env SDKROOT="/" /b/s/w/ir/k/staging/llvm_build/bin/clang -emit-llvm -S -o /b/s/w/ir/k/staging/llvm_build/tools/clang/test/Frontend/Output/ast-main.c.tmp1.ll -x c - < /b/s/w/ir/k/llvm-project/clang/test/Frontend/ast-main.c
: 'RUN: at line 2';   env SDKROOT="/" /b/s/w/ir/k/staging/llvm_build/bin/clang -emit-ast -o /b/s/w/ir/k/staging/llvm_build/tools/clang/test/Frontend/Output/ast-main.c.tmp.ast /b/s/w/ir/k/llvm-project/clang/test/Frontend/ast-main.c
: 'RUN: at line 3';   env SDKROOT="/" /b/s/w/ir/k/staging/llvm_build/bin/clang -emit-llvm -S -o /b/s/w/ir/k/staging/llvm_build/tools/clang/test/Frontend/Output/ast-main.c.tmp2.ll -x ast - < /b/s/w/ir/k/staging/llvm_build/tools/clang/test/Frontend/Output/ast-main.c.tmp.ast
: 'RUN: at line 4';   diff /b/s/w/ir/k/staging/llvm_build/tools/clang/test/Frontend/Output/ast-main.c.tmp1.ll /b/s/w/ir/k/staging/llvm_build/tools/clang/test/Frontend/Output/ast-main.c.tmp2.ll
--
Exit Code: 1
Aug 18 2020, 9:45 PM · Restricted Project

@keith do you plan on finishing this? I think we should change the name from -fcoverage-prefix-map to -fprofile-prefix-map, but otherwise it might be good to go.

Aug 18 2020, 7:02 PM · Restricted Project

I created https://sourceware.org/bugzilla/show_bug.cgi?id=26404 for the discussion on the syntax.

@christylee @phosek You may consider starting a thread on binutils@sourceware.org to expedite their processing:)

Aug 18 2020, 5:23 PM · Restricted Project

LGTM

Aug 18 2020, 3:55 PM · Restricted Project
phosek added a comment to D76482: [lld][ELF] Provide optional hidden symbols for build ID.

D76482 is an example use case for this feature.

Aug 18 2020, 3:36 PM · Restricted Project

LGTM

Aug 18 2020, 3:15 PM · Restricted Project, Restricted Project
phosek added inline comments to D86134: Fix OCaml build failure because of absolute path in system libs.
Aug 18 2020, 12:52 PM · Restricted Project

# Aug 17 2020

LGTM

Aug 17 2020, 11:01 AM · Restricted Project, Restricted Project, Restricted Project

@ldionne this is still failing and it's been several days now, can we revert this change?

# Aug 15 2020

Note that this issue isn't manifesting with CMake 3.17 and newer.

# Aug 13 2020

LGTM

Aug 13 2020, 2:53 PM · Restricted Project

# Aug 12 2020

phosek added a comment to D85706: [compiler-rt] Compile assembly files as ASM not C.

One concern I have with this change is that we may not always consistently update CMAKE_ASM_FLAGS or set CMAKE_ASM_COMPILER. This wouldn't make a difference before, but it'll with this change. Can you check if these variables are being updated as needed?

Aug 12 2020, 2:25 PM · Restricted Project, Restricted Project
phosek added a comment to D85820: Use find_library for ncurses.

CMake already has FindCurses module so we could consider using find_package(Curses), unfortunately it doesn't define the imported target. I'd also prefer replacing HAVE_TERMINFO with LLVM_ENABLE_TERMINFO everywhere for consistency with zlib or libxml2 (see D84563).

Aug 12 2020, 12:59 PM · Restricted Project, Restricted Project, Restricted Project
phosek added inline comments to D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use.
Aug 12 2020, 12:54 PM · Restricted Project

LGTM

Aug 12 2020, 12:33 PM · Restricted Project

# Aug 11 2020

phosek added a comment to D85678: [elfabi] Move elfabi related code to InterfaceStub library.

I'd be fine with InterfaceStub as a name for the library.

@phosek @haowei Are there any plans in the near term to unify any of the TBD work with InterfaceStubs/elftapi?

Aug 11 2020, 11:32 PM · Restricted Project