inglorion (Bob Haarman)
User

Projects

User does not belong to any projects.
User Since
Dec 15 2015, 10:55 AM (67 w, 12 h)

Recent Activity

Yesterday

inglorion added a comment to D31444: LTO: call getRealLinkageName on IRNames before feeding to getGUID.

@mehdi_amini, we have other places where we call GlobalValue::getGUID with a name that is not an IR name, e.g. in BitcodeReader.cpp. I wouldn't be opposed to moving to a design where it's harder to pass the wrong kind of name, or even a design where the IR name and the RealLinkageName and the GlobalIdentifier aren't all different things, but that is going to be a larger change. Can we get this fix in in the meantime?

Tue, Mar 28, 4:38 PM
inglorion added a comment to D31444: LTO: call getRealLinkageName on IRNames before feeding to getGUID.

Thanks, pcc, for helping debug this, and rnk for the suggestion to use getRealLinkageName to solve it.

Tue, Mar 28, 4:22 PM
inglorion created D31444: LTO: call getRealLinkageName on IRNames before feeding to getGUID.
Tue, Mar 28, 4:20 PM
inglorion committed rL298942: COFF: include archive name in LTO object name.
COFF: include archive name in LTO object name
Tue, Mar 28, 2:32 PM
inglorion closed D31402: COFF: include archive name in LTO object name by committing rL298942: COFF: include archive name in LTO object name.
Tue, Mar 28, 2:32 PM
inglorion updated the diff for D31402: COFF: include archive name in LTO object name.

added test

Tue, Mar 28, 1:58 PM

Mon, Mar 27

inglorion planned changes to D31402: COFF: include archive name in LTO object name.

Adding the offset also affects non-LTO archive members, right? If so, I prefer to fix that in a follow up. For this one, I'd like to change ParentName + MB.getBufferIdentifier() to toString(this), which is more consistent with how we form the names in other places. I will also add a test.

Mon, Mar 27, 11:39 AM
inglorion created D31402: COFF: include archive name in LTO object name.
Mon, Mar 27, 10:43 AM

Wed, Mar 22

inglorion committed rL298525: [compiler-rt] build compiler-rt runtimes without LTO.
[compiler-rt] build compiler-rt runtimes without LTO
Wed, Mar 22, 10:38 AM
inglorion closed D31218: [compiler-rt] build compiler-rt runtimes without LTO by committing rL298525: [compiler-rt] build compiler-rt runtimes without LTO.
Wed, Mar 22, 10:38 AM

Tue, Mar 21

inglorion updated the diff for D31218: [compiler-rt] build compiler-rt runtimes without LTO.

added comment, as suggested by @davidxl

Tue, Mar 21, 4:01 PM
inglorion added a comment to D31218: [compiler-rt] build compiler-rt runtimes without LTO.

@rnk, thanks for taking a look. I have an alternative implementation that records which flags were added to enable LTO and provides functions to filter those out. That might lead to cleaner command lines. On the other hand, this is simpler and follows what we already do for SANITIZER_COMMON_FLAGS (in compiler-rt/CMakeLists.txt), so I decided to see what people think about this implementation, first.

Tue, Mar 21, 3:40 PM
inglorion created D31218: [compiler-rt] build compiler-rt runtimes without LTO.
Tue, Mar 21, 3:28 PM
inglorion added a comment to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.

Thanks for reviewing!

Tue, Mar 21, 11:38 AM
inglorion committed rL298413: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.
[compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests
Tue, Mar 21, 11:37 AM
inglorion closed D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests by committing rL298413: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.
Tue, Mar 21, 11:37 AM

Mon, Mar 20

inglorion added inline comments to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.
Mon, Mar 20, 4:06 PM
inglorion updated the diff for D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.

apply similar change to darwin_test_archs

Mon, Mar 20, 4:05 PM
inglorion added inline comments to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.
Mon, Mar 20, 3:41 PM
inglorion added a comment to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.

The "error adding symbols" message means we're still not using the right linker (it comes from ld, in a build where lld should be used as the linker).

Mon, Mar 20, 2:57 PM
inglorion added a comment to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.

With this new diff, check-profile runs the same number of tests as it does in the base revision (102, in my case). Unfortunately, when building with thinlto, they all fail (error message: llvm-build-clang/lib/clang/5.0.0/lib/linux/libclang_rt.profile-i386.a: error adding symbols: File format not recognized).

Mon, Mar 20, 2:55 PM
inglorion updated the diff for D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.

Changed test_target_arch to pass CMAKE_EXE_LINKER_FLAGS and argstring, which matches what it did before.

Mon, Mar 20, 2:51 PM
inglorion added inline comments to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.
Mon, Mar 20, 2:34 PM
inglorion added a comment to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.

I'm seeing 100 tests passed, 2 unsupported without this change, and 50 passed, 1 unsupported with the change. That doesn't seem right.

Mon, Mar 20, 1:32 PM
inglorion added a comment to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.

Thanks, davidxl. With your command line I do get the check-profile target.

Mon, Mar 20, 12:43 PM
inglorion added a comment to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.

I configured the build using

Mon, Mar 20, 10:38 AM

Fri, Mar 17

inglorion added a comment to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.

@davidxl: My ninja doesn't know about check-profile. Did you mean check-clang-profile? That gives me 32 tests with and without this change.

Fri, Mar 17, 5:44 PM
inglorion committed rL298124.
Fri, Mar 17, 2:45 PM
inglorion closed D31011: recommend using llvm-ar when finding undefined references and empty archives by committing rL298124.
Fri, Mar 17, 2:45 PM
inglorion updated the diff for D31011: recommend using llvm-ar when finding undefined references and empty archives.

clang-format done right and removed NoSymbols local

Fri, Mar 17, 2:13 PM
inglorion added a comment to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.

Some more background on this change:

Fri, Mar 17, 1:38 PM
inglorion added a comment to D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.

@beanz, @davidxl : It isn't clear to me from the comment that was in CMakeLists.txt what exact problem CMP0056 NEW caused with test_target_arch, so I'm not completely sure what I did avoids the problem. Could one of you check if my change seems ok or if there is a better way?

Fri, Mar 17, 1:32 PM
inglorion created D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests.
Fri, Mar 17, 1:30 PM

Thu, Mar 16

inglorion added inline comments to D31011: recommend using llvm-ar when finding undefined references and empty archives.
Thu, Mar 16, 4:19 PM
inglorion added a comment to D31011: recommend using llvm-ar when finding undefined references and empty archives.

Yes, I'll check Chromium builds once we're happy with the implementation.

Thu, Mar 16, 3:34 PM

Wed, Mar 15

inglorion updated the diff for D31011: recommend using llvm-ar when finding undefined references and empty archives.

@ruiu's comments

Wed, Mar 15, 5:21 PM
inglorion updated the diff for D31011: recommend using llvm-ar when finding undefined references and empty archives.

@ruiu, how is this?

Wed, Mar 15, 5:05 PM
inglorion added a comment to D31011: recommend using llvm-ar when finding undefined references and empty archives.

@ruiu: Yes, that would be a lot simpler. Let me explain why I did it the way I did it, and then I'll be happy to simplify it if you still feel that's better.

Wed, Mar 15, 4:47 PM
inglorion added inline comments to D31011: recommend using llvm-ar when finding undefined references and empty archives.
Wed, Mar 15, 4:19 PM
inglorion created D31011: recommend using llvm-ar when finding undefined references and empty archives.
Wed, Mar 15, 4:08 PM
inglorion accepted D30956: Introduce NativeEnumModules and NativeCompilandSymbol.

lgtm2

Wed, Mar 15, 12:58 PM
inglorion accepted D30959: [pdb] Add support for writing Module Info and module symbols.

Thanks for the clarifications! lgtm

Wed, Mar 15, 12:54 PM
inglorion added inline comments to D30956: Introduce NativeEnumModules and NativeCompilandSymbol.
Wed, Mar 15, 11:38 AM
inglorion added a comment to D30959: [pdb] Add support for writing Module Info and module symbols.

Thanks for making it easy to write minimal PDBs! I added a couple of inline comments about the code.

Wed, Mar 15, 11:18 AM

Tue, Mar 7

inglorion accepted D30663: Use filename in linemarker when compiling preprocessed source (Revised).

Fixing the other issues in a follow-up seems fine. This lgtm.

Tue, Mar 7, 11:25 AM

Mon, Mar 6

inglorion added inline comments to D30663: Use filename in linemarker when compiling preprocessed source (Revised).
Mon, Mar 6, 10:51 AM

Fri, Mar 3

inglorion added a comment to D30591: Introduce the feature "linux" for tests only for linux.

Checking for linux when really you want to check for ELF doesn't seem right. In this case, I think there is an better way to do it; instead of relying on llvm-objdump, could you emit an LLVM assembly file and check that for presence of the string you want? I think if you compile with clang -g -S -emit-llvm, it will give you LLVM assembly with metadata for the records you need and you won't need to generate an object file.

Fri, Mar 3, 3:27 PM

Wed, Mar 1

inglorion committed rL296658: enable building with LTO on Windows using clang-cl and lld.
enable building with LTO on Windows using clang-cl and lld
Wed, Mar 1, 11:34 AM
inglorion closed D30240: enable building with LTO on Windows using clang-cl and lld by committing rL296658: enable building with LTO on Windows using clang-cl and lld.
Wed, Mar 1, 11:34 AM

Tue, Feb 28

inglorion added inline comments to D30240: enable building with LTO on Windows using clang-cl and lld.
Tue, Feb 28, 3:51 PM
inglorion updated the diff for D30240: enable building with LTO on Windows using clang-cl and lld.

refactored check for lld-link to be more explicit and less confusing

Tue, Feb 28, 3:50 PM

Mon, Feb 27

inglorion added inline comments to D30240: enable building with LTO on Windows using clang-cl and lld.
Mon, Feb 27, 3:25 PM
inglorion updated the diff for D30240: enable building with LTO on Windows using clang-cl and lld.

added error message for when unsupported combination of platform, LTO, and linker is used

Mon, Feb 27, 2:10 PM
inglorion added a comment to D30363: COFF ICF: Merge only functions. Do not merge read-only data..

This causes us to eliminate less duplication, right? Is folding of identical non-functions something we may want to re-enable at a later point? Should we make the behavior controllable by a flag - or perhaps only implement the MSVC-compatible behavior when using msvclto?

Mon, Feb 27, 12:00 PM
inglorion committed rL296373: enable -flto=thin in clang-cl.
enable -flto=thin in clang-cl
Mon, Feb 27, 11:52 AM
inglorion closed D30239: enable -flto=thin in clang-cl by committing rL296373: enable -flto=thin in clang-cl.
Mon, Feb 27, 11:52 AM
inglorion added inline comments to D30348: De-template DefinedRegular..
Mon, Feb 27, 11:44 AM

Feb 23 2017

inglorion updated the diff for D30239: enable -flto=thin in clang-cl.

changed error message

Feb 23 2017, 6:04 PM
inglorion updated the diff for D30239: enable -flto=thin in clang-cl.

fail early with a friendlier message when using -flto without -fuse-ld=lld

Feb 23 2017, 4:52 PM
inglorion updated the diff for D30239: enable -flto=thin in clang-cl.

added missing --

Feb 23 2017, 4:14 PM
inglorion added inline comments to D30239: enable -flto=thin in clang-cl.
Feb 23 2017, 4:01 PM
inglorion committed rL296042: [COFF] added test for thinlto.
[COFF] added test for thinlto
Feb 23 2017, 3:54 PM
inglorion closed D30277: [COFF] added test for thinlto by committing rL296042: [COFF] added test for thinlto.
Feb 23 2017, 3:54 PM
inglorion updated the diff for D30239: enable -flto=thin in clang-cl.

Implemented @hans's suggestion of moving the tests into cl-options.c.

Feb 23 2017, 3:42 PM
inglorion updated the diff for D30277: [COFF] added test for thinlto.

check for absence of undefined foo, instead of presence of main

Feb 23 2017, 2:00 PM
inglorion updated the summary of D30277: [COFF] added test for thinlto.
Feb 23 2017, 1:12 PM
inglorion updated the diff for D30277: [COFF] added test for thinlto.

use llvm-nm instead of looking at the bitcode

Feb 23 2017, 1:12 PM
inglorion added inline comments to D30240: enable building with LTO on Windows using clang-cl and lld.
Feb 23 2017, 11:44 AM
inglorion added a comment to D30277: [COFF] added test for thinlto.

Nit: I'd use llvm-nm to check the symbol tables of the object files so that we're checking the inputs that the linker actually reads, rather than a byproduct of LTO.

Feb 23 2017, 11:30 AM

Feb 22 2017

inglorion added inline comments to D30277: [COFF] added test for thinlto.
Feb 22 2017, 4:19 PM
inglorion updated the diff for D30277: [COFF] added test for thinlto.

@pcc: How is this instead?

Feb 22 2017, 4:15 PM
inglorion created D30277: [COFF] added test for thinlto.
Feb 22 2017, 2:54 PM
inglorion committed rL295872: stop using associative comdats for SEH filter functions.
stop using associative comdats for SEH filter functions
Feb 22 2017, 12:41 PM
inglorion closed D30117: stop using associative comdats for SEH filter functions by committing rL295872: stop using associative comdats for SEH filter functions.
Feb 22 2017, 12:41 PM
inglorion added a comment to D30239: enable -flto=thin in clang-cl.

Is clang-cl using lld as default? How is the switch done? Ideally we should have a nice error message from the driver if -flto is used without lld.

Feb 22 2017, 10:31 AM

Feb 21 2017

inglorion added a dependency for D30240: enable building with LTO on Windows using clang-cl and lld: D30239: enable -flto=thin in clang-cl.
Feb 21 2017, 8:12 PM
inglorion added a dependent revision for D30239: enable -flto=thin in clang-cl: D30240: enable building with LTO on Windows using clang-cl and lld.
Feb 21 2017, 8:12 PM
inglorion created D30240: enable building with LTO on Windows using clang-cl and lld.
Feb 21 2017, 8:11 PM
inglorion added a dependency for D30239: enable -flto=thin in clang-cl: D30117: stop using associative comdats for SEH filter functions.
Feb 21 2017, 7:58 PM
inglorion added a dependent revision for D30117: stop using associative comdats for SEH filter functions: D30239: enable -flto=thin in clang-cl.
Feb 21 2017, 7:58 PM
inglorion created D30239: enable -flto=thin in clang-cl.
Feb 21 2017, 7:58 PM

Feb 17 2017

inglorion committed rL295507: [COFF] support /ERRORLIMIT option.
[COFF] support /ERRORLIMIT option
Feb 17 2017, 2:57 PM
inglorion closed D29691: [COFF] support /ERRORLIMIT option by committing rL295507: [COFF] support /ERRORLIMIT option.
Feb 17 2017, 2:57 PM
inglorion updated the diff for D29691: [COFF] support /ERRORLIMIT option.

changes requested by @ruiu

Feb 17 2017, 2:31 PM
inglorion created D30117: stop using associative comdats for SEH filter functions.
Feb 17 2017, 2:31 PM
inglorion added a comment to D30117: stop using associative comdats for SEH filter functions.

With this change, LLVM, Clang, and lld compile using LTO and pass tests on Windows. Without the change, the tests fail to compile ("Associative COMDAT symbol ... does not exist", https://bugs.llvm.org//show_bug.cgi?id=31974).

Feb 17 2017, 2:31 PM

Feb 9 2017

inglorion committed rL294664: added missing "savetemps/" in test/COFF/savetemps.ll.
added missing "savetemps/" in test/COFF/savetemps.ll
Feb 9 2017, 3:43 PM
inglorion committed rL294643: list paths explicitly instead of using * in.
list paths explicitly instead of using * in
Feb 9 2017, 3:04 PM
inglorion closed D29788: list paths explicitly instead of using * in test/COFF/savetemps.ll by committing rL294643: list paths explicitly instead of using * in.
Feb 9 2017, 3:04 PM
inglorion updated the diff for D29788: list paths explicitly instead of using * in test/COFF/savetemps.ll.

use a subdirectory

Feb 9 2017, 2:28 PM
inglorion added a comment to D29788: list paths explicitly instead of using * in test/COFF/savetemps.ll.

How about creating a temporary subdirectory (e.g. %T/tmp/) and remove that directory entirely?

Feb 9 2017, 2:22 PM
inglorion created D29788: list paths explicitly instead of using * in test/COFF/savetemps.ll.
Feb 9 2017, 2:07 PM

Feb 8 2017

inglorion committed rL294498: [COFF] added support for /lldsavetemps.
[COFF] added support for /lldsavetemps
Feb 8 2017, 10:48 AM
inglorion closed D29518: [COFF] added support for /lldsavetemps by committing rL294498: [COFF] added support for /lldsavetemps.
Feb 8 2017, 10:48 AM
inglorion committed rL294497: [sanitizer] if WINAPI is already defined, do not redefine it.
[sanitizer] if WINAPI is already defined, do not redefine it
Feb 8 2017, 10:45 AM
inglorion closed D29683: [sanitizer] if WINAPI is already defined, do not redefine it by committing rL294497: [sanitizer] if WINAPI is already defined, do not redefine it.
Feb 8 2017, 10:45 AM

Feb 7 2017

inglorion added a comment to D29683: [sanitizer] if WINAPI is already defined, do not redefine it.

Including minwindef.h makes the compiler very unhappy, complaining about a lot of undefined types and identifiers. I guess if we wanted to include something, we would have to include something much larger. I think there is a good argument to be made to just define the one macro we care about here. Ok to ship as-is?

Feb 7 2017, 4:14 PM
inglorion updated the diff for D29518: [COFF] added support for /lldsavetemps.

added comment

Feb 7 2017, 4:09 PM
inglorion created D29691: [COFF] support /ERRORLIMIT option.
Feb 7 2017, 4:08 PM
inglorion added inline comments to D29518: [COFF] added support for /lldsavetemps.
Feb 7 2017, 3:18 PM