[PATCH] D121838: Generalize "check-all" umbrella targets, use for check-clang-tools

JamesNagurne added a comment. In D121838#3543421 , @JamesNagurne wrote: > After some investigation, I found that we did not set LLVM_INCLUDE_TESTS from > the top-level project. This seems like an oversight because we build the > test-depends (and check

[PATCH] D121838: Generalize "check-all" umbrella targets, use for check-clang-tools

JamesNagurne added a comment. After some investigation, I found that we did not set LLVM_INCLUDE_TESTS from the top-level project. This seems like an oversight because we build the test-depends (and check-all) regularly. After seeing LLVM_INCLUDE_TESTS to ON, the builds worked. This commit clea

[PATCH] D121838: Generalize "check-all" umbrella targets, use for check-clang-tools

JamesNagurne added a comment. Hi Sam, I've got a downstream build system that is failing here and I'm trying to figure out why. After your commit, I get an error during bootstrapped runtime builds: 05-27 14:41 __main__ INFO [202/796] cd /path/to/builddir/runtimes/runtimes-target-bins

[PATCH] D110673: [clang] Don't modify OptRemark if the argument is not relevant

JamesNagurne added a comment. You're welcome! Surprising that no downstream clang was running with the old PM and assertions... I guess we better get on the new PM soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110673/new/ https://reviews.ll

[PATCH] D110673: [clang] Don't modify OptRemark if the argument is not relevant

JamesNagurne added a comment. Perhaps you could reproduce my error -round-trip-args on the -cc1 command line? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110673/new/ https://reviews.llvm.org/D110673 __

[PATCH] D110673: [clang] Don't modify OptRemark if the argument is not relevant

JamesNagurne added a comment. The trigger for the remark I'm seeing is llvm::shouldInline in InlineAdvisor.cpp ((https://github.com/llvm/llvm-project/blob/2240deb9766cc080b351016b0d7f975d7249b113/llvm/lib/Transforms/IPO/Inliner.cpp#L425) which is called in a top-level AlwaysInlinerLegacyPass C

[PATCH] D110673: [clang] Don't modify OptRemark if the argument is not relevant

JamesNagurne added a comment. I'll take a quick look tomorrow, but the general idea is that on calling ParseOptimizationRemark on line 1909 with a -cc1 command line containing -Rpass=inline -Rno-pass, Opts.OptimizationRemarkMissed and Opts.OptimizationRemarkAnalysis are set to valid patterns (R

[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute

JamesNagurne added a comment. In D83174#2230763 , @aaron.ballman wrote: > Unfortunately, I had to revert the change as it was causing some buildbots to > fail. I reverted in 58c305f466d1f78adb10e7295b9bc9fc192a6e09 >

[PATCH] D68897: [clang][ifs] Avoid assumption of default visibility in InterfaceStubs tests

JamesNagurne added a comment. Sadly I'm home from the weekend, so I can't post more diffs for the renaming. Here's a link to a github search though! I think this would be all the places: https://github.com/llvm/llvm-project/search?q=iterface&unscoped_q=iterface I nor the rest of my team have the

[PATCH] D63978: Clang Interface Stubs merger plumbing for Driver

JamesNagurne added a comment. In D63978#1706714 , @plotfi wrote: > In D63978#1706502 , @JamesNagurne > wrote: > > > In D63978#1706448 , @plotfi wrote: > > > > > In D63978#17

[PATCH] D68897: [clang][ifs] Avoid assumption of default visibility in InterfaceStubs tests

JamesNagurne created this revision. JamesNagurne added reviewers: plotfi, snidertm. Herald added a project: clang. Herald added a subscriber: cfe-commits. In D63978 , tests were added which expect that the default visibility is maintained without an explicit optio

[PATCH] D63978: Clang Interface Stubs merger plumbing for Driver

JamesNagurne added a comment. In D63978#1706448 , @plotfi wrote: > In D63978#1706420 , @JamesNagurne > wrote: > > > Our team maintains a downstream embedded ARM clang distribution and some > > tests from this comm

[PATCH] D63978: Clang Interface Stubs merger plumbing for Driver

JamesNagurne added a comment. Our team maintains a downstream embedded ARM clang distribution and some tests from this commit have begun to fail for us. For a number of these tests, there was a REQUIRES: x86-registered-target at the top, which has now been removed. Specifically, externstatic.c,

[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

JamesNagurne added a comment. In D65907#1645474 , @arphaman wrote: > In D65907#1643800 , @JamesNagurne > wrote: > > > In D65907#1643650 , @arphaman > > wrote: > > > > > No

[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

JamesNagurne added a comment. In D65907#1643650 , @arphaman wrote: > No the windows test failure was different, there were no Deps at all. I'm > currently investigating it on a windows VM. > > @JamesNagurne I think there's some issue with the working dire

[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

JamesNagurne added a comment. Herald added a subscriber: ributzka. @arphaman you disabled this test on Windows, but did not specify exactly how it fails. My team works on an embedded ARM compiler (most similar to arm-none-eabi), and we're now seeing failures from DependencyScannerTest. I can't f

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

JamesNagurne added a comment. In case you haven't seen, this commit breaks non-x86 build bots due to the combination of '-triple x86_64*' and '-S'. Some tests that use this target are only looking for AST dumps, and do not actually require such a target. This is not one of those tests, as it's

[PATCH] D64653: clang/test/Driver/fsanitize.c: Fix -fsanitize=vptr using default target

JamesNagurne added a comment. In D64653#1584155 , @MaskRay wrote: > I committed this for you in rL365981 (it > may have broken a Windows build for a long time. I wanted to fix it soon..). > Thanks! > > > any platform t

[PATCH] D64653: clang/test/Driver/fsanitize.c: Fix -fsanitize=vptr using default target

JamesNagurne created this revision. JamesNagurne added a reviewer: MaskRay. Herald added a project: clang. Herald added a subscriber: cfe-commits. test/Driver/fsanitize.c: Fix -fsanitize=vptr using default target In revision rL365872 , a line containing '-targe

[PATCH] D60691: [ARM] Replace fp-only-sp and d16 with fp64 and d32.

JamesNagurne added a comment. Hi Simon et. al., I'm working on a downstream ARM toolchain and have downstreamed this change into our codebase. We saw that you've fixed the -mfpu=none issue and have taken that as well, but are still running into some issues. Prior to your change, the optionset "