[PATCH] D72390: [www] Remove stale text about default c++ standard from cxx_status.html

2023-10-05 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop abandoned this revision. russell.gallop added a comment. Herald added a project: All. Latest page looks fine: https://clang.llvm.org/cxx_status.html#cxx11 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72390/new/

[PATCH] D131665: [CMake] Support passing arguments to build tool (bootstrap).

2022-08-22 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop resigned from this revision. russell.gallop added a comment. Resign as reviewer as I work with Carlos (and am not familiar enough with the details of this area). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131665/new/

[PATCH] D131665: [CMake] Support passing arguments to build tool (bootstrap).

2022-08-11 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. Noting related review: https://reviews.llvm.org/D115815 which added this variable to support this for other "external projects". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131665/new/

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-03-09 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop abandoned this revision. russell.gallop added a comment. In D96120#2597877 , @kcc wrote: > I am afraid we will have to delete the older (non-standalone) variant > entirely. > (And the sooner the better) Okay, think we found a few things

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-03-01 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D96120#2591368 , @mstorsjo wrote: > Yes, it would need something similar - I tried whipping something together, > which after some tweaks seems to work: > Feel free to squash that into your patch (which saves me a bit

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-03-01 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 327123. russell.gallop added a comment. Update with suggested changes to MinGW.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96120/new/ https://reviews.llvm.org/D96120 Files: clang/lib/Driver/ToolChains/MSVC.cpp

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-27 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D96120#2591418 , @vitalybuka wrote: >>> This is intended as a step to porting scudo standalone. > > Why this is needed for scudo stadalone? It’s not strictly needed. It seemed like it was easier, as some of the work had

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-26 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop marked 2 inline comments as done. russell.gallop added a comment. In D96120#2550941 , @mstorsjo wrote: > In D96120#2550876 , @russell.gallop > wrote: > >> In D96120#2546077

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-26 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 326716. russell.gallop added a comment. Added comment on AllocatorSize. Applied Mingw changes suggested by @mstorsjo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96120/new/

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-09 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D96120#2545151 , @aganea wrote: > A few questions: Does this work on x86 targets? I haven't tried. I'll test this out. > Are the scudo tests below being built with /MD or with /MT? The lit tests? They don't specify

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-09 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D96120#2546077 , @mstorsjo wrote: > As is, this breaks compilation for mingw. With the three modifications I > suggest here, it no longer breaks compilation for me - I have no idea if it > actually works in mingw

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2021-02-05 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. I believe that the MCJIT failures are due to bug https:/bugs.llvm.org/show_bug.cgi?id=24978 rather than a problem in the Scudo port so I have added details of how I hit it to that bugzilla and opened two reviews to get this landed:

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-02-05 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added reviewers: cryptoad, aganea. Herald added subscribers: phosek, mgorny. russell.gallop requested review of this revision. Herald added projects: clang, Sanitizers. Herald added subscribers: Sanitizers, cfe-commits. Based on

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2021-02-03 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. I've focussed on the test test-global-init-nonzero-sm-pic.ll which fails writing to an address which (I believe) should be in the .data section but isn't. With some breakpoints in SectionMemoryManager.cpp it appears that this fails when the top 32bits of the

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2021-02-02 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. I managed to get this to fail in the debugger (for the cross-module-sm-pic-a.ll test): 01655e9d001e() Unknown 01655e9d0019() Unknown 017e5eb6b410() Unknown 017c5eb63810() Unknown

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2021-02-02 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 320806. russell.gallop added a comment. Remove -fsanitize=scudo support for Windows in LLVM cmake, using LLVM_INTEGRATED_CRT_ALLOC instead. Remove scudo_cxx from LLVM_INTEGRATED_CRT_ALLOC. CHANGES SINCE LAST ACTION

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-12-18 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 312827. russell.gallop edited the summary of this revision. russell.gallop added a comment. Apologies for the delay, I've had other things taking my time. Latest version uploaded. This fixes stage1 lit tests (on Windows and Linux) and adds scudo_cxx

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-10-28 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 301331. russell.gallop added a comment. Herald added a subscriber: dexonsmith. Apologies for the delay, I've had a few other things on. I've worked through the tests. Several I've marked as unsupported on windows as they use things unixy things like

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-09-23 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D86694#2288660 , @aganea wrote: > I'm also in favor, I think this is good direction ahead. It'd be nice if > following issues were fixed -- in subsequent patches if you wish: > > - Stage1 `ninja check-scudo` fails many

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-09-22 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. > I'm going to concentrate on the standalone port as I think that's the way > forward. If I get that working then can work through the other issues. I'm considering a slight change of plan. @cryptoad, in the name of incremental development, would you be happy

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-09-15 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D86694#2274383 , @cryptoad wrote: > In D86694#2273815 , @aganea wrote: > >> If 4.4 TB of virtual pages are mapped in each process (this happens on >> startup), then we quickly

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-09-15 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D86694#2273815 , @aganea wrote: > @russell.gallop I see a lots of failing tests when running `ninja check-all` > on a Scudo-enabled build (stage 2). Do you see the same thing on your end? Not a lot but a few, including

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-09-14 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D86694#2270433 , @aganea wrote: > Thanks for working on this @russell.gallop! > > I've reproduced your tests, please see below. The only difference is that > I've used a ThinLTO build for stage2: Thanks. It's good to

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-09-14 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 291511. russell.gallop added a comment. Re-upload patch with G LLVM Github Monorepo set. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86694/new/ https://reviews.llvm.org/D86694 Files:

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-09-11 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D86694#2242559 , @cryptoad wrote: > In D86694#2242150 , @russell.gallop > wrote: > >> In D86694#2242140 , @cryptoad wrote: >> >>> That's

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-09-11 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 291235. russell.gallop edited the summary of this revision. russell.gallop added a comment. Herald added subscribers: phosek, hiraditya. Fixup scudo (sanitizer based) to work on Windows. This makes use of the CRT alloc hooks from D71786

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-08-27 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D86694#2242150 , @russell.gallop wrote: > You marked D42519 as WIP, can you remember > what was still TBD? Maybe more tests could be enabled... Repository: rG LLVM Github Monorepo

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-08-27 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D86694#2242140 , @cryptoad wrote: > That's awesome! Is it meant to eventually be committed or only be used for > comparison purposes? I'd like it to be committed, but can't claim I know the code from

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

2020-08-27 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added reviewers: cryptoad, aganea, hans. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, mgorny. Herald added projects: clang, Sanitizers, LLVM. russell.gallop requested review of this revision. This is basically

[PATCH] D79147: Switch to using -debug-info-kind=constructor as default (from =limited)

2020-07-28 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. Hi, It looks like is causing one of the debuginfo-tests: llgdb-tests/nrvo-string.cpp to fail, run on Linux. Failure as below. I don't think the debuginfo-tests are run on any bot (but probably should be!). I bisected the failure back to this change. Please

[PATCH] D78454: [clangd] Highlight related control flow.

2020-05-28 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. Hi Sam, It looks like this is causing a failure on the Windows PS4 buildbot: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/32606 Please could you take a look? PS4 target disables RTTI, hence exceptions, by default so

[PATCH] D78030: [TimeProfiler] Emit clock synchronization point

2020-04-22 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop accepted this revision. russell.gallop added a comment. LGTM, with a few small comments. For the record, I wondered whether the time profiler could emit all ts as absolute, so I tried it out. This has two problems. 1). The "Total" numbers also need adjusting to be at the

[PATCH] D78027: [TimeProfiler] Emit real process ID and thread names

2020-04-14 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop accepted this revision. russell.gallop added a comment. This revision is now accepted and ready to land. Would be nice if the test could check that the pid was being set to something but not sure how you could do that (beyond checking that it's a number). Apart from that, LGTM.

[PATCH] D69043: [RFC] Adding time-trace to LLD?

2020-02-10 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop abandoned this revision. russell.gallop added a comment. This was submitted as the sequence of patches: https://reviews.llvm.org/D70904 - Tidying up in TimeProfiler.cpp https://reviews.llvm.org/D70950 - Add ProcName to TimeTraceProfiler https://reviews.llvm.org/D71059 - [LLD][ELF]

[PATCH] D70950: Add ProcName to TimeTraceProfiler

2020-01-31 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D70950#1850796 , @rnk wrote: > This broke ClangBuildAnalyzer on Windows because it has a very naive check > for "clang": > https://github.com/aras-p/ClangBuildAnalyzer/blob/master/src/main.cpp#L148 > I was wondering

[PATCH] D73162: [test] Avoid loop-unroll.c test getting confused by fadd in git revision

2020-01-23 Thread Russell Gallop via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4662f6e1c778: [test] Avoid loop-unroll.c test getting confused by fadd in git revision (authored by russell.gallop). Changed prior to commit: https://reviews.llvm.org/D73162?vs=239808=239812#toc

[PATCH] D73162: [test] Avoid loop-unroll.c test getting confused by fadd in git revision

2020-01-23 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D73162#1834379 , @asbirlea wrote: > Oh, wow! Might I ask you add the same for fmul? We may get a revision like > that next time :). > Thank you! fmul shouldn't have the same problem as git hashes are only hex

[PATCH] D73162: [test] Avoid loop-unroll.c test getting confused by fadd in git revision

2020-01-23 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 239808. russell.gallop added a comment. Update to check for start of metadata. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73162/new/ https://reviews.llvm.org/D73162 Files: clang/test/CodeGen/loop-unroll.c Index:

[PATCH] D73162: [test] Avoid loop-unroll.c test getting confused by fadd in git revision

2020-01-22 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added a reviewer: asbirlea. Herald added a subscriber: zzheng. Herald added a project: clang. Saw this test failing as it was matching fadd in a (local) git revision: F:\git\llvm-project\clang\test\CodeGen\loop-unroll.c:38:30: error:

[PATCH] D73123: [CMake] Fixes for including LLVM into another CMake project

2020-01-21 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added reviewers: chandlerc, dcoughlin, jroelofs, mgorny, pcc, tstellar, beanz, dsanders. Herald added subscribers: cfe-commits, Charusso. Herald added projects: clang, LLVM. This fixes a couple of paths to allow add_subdirectory(llvm) from a

[PATCH] D72390: [www] Remove stale text about default c++ standard from cxx_status.html

2020-01-15 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. Ping. Is this okay to land, or have I missed something? Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72390/new/ https://reviews.llvm.org/D72390 ___ cfe-commits

[PATCH] D72390: [www] Remove stale text about default c++ standard from cxx_status.html

2020-01-08 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added reviewers: rsmith, t.p.northover. Herald added a project: clang. Since r320250 c++98 is no longer the default, but cxx_status.html still said it was. I think the original intent of the statement was to say that clang by default supports

[PATCH] D71347: Add TimeTraceScope constructor without detail arg to simplify code where no detail required

2019-12-11 Thread Russell Gallop via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGdf494f7512b0: [Support] Add TimeTraceScope constructor without detail arg (authored by russell.gallop). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71347: Add TimeTraceScope constructor without detail arg to simplify code where no detail required

2019-12-11 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added a reviewer: anton-afanasyev. Herald added subscribers: cfe-commits, arphaman, hiraditya. Herald added projects: clang, LLVM. Also don't write out detail when it is empty, which reduces size of generated traces. Repository: rG LLVM

[PATCH] D70950: Add ProcName to TimeTraceProfiler

2019-12-03 Thread Russell Gallop via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGaedeab7f85ca: [Support] Add ProcName to TimeTraceProfiler (authored by russell.gallop). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70950/new/

[PATCH] D70950: Add ProcName to TimeTraceProfiler

2019-12-03 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added a reviewer: anton-afanasyev. Herald added subscribers: cfe-commits, hiraditya. Herald added projects: clang, LLVM. This was hard-coded to "clang". This change allows it to to be used on processes other than clang (such as lld). This

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

2019-12-03 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. Thanks for the updated timings. I have no objection to this going in. I haven't gone through the code changes in detail so I think you should probably get approval from someone who has (such as @hans). CHANGES SINCE LAST ACTION

[PATCH] D69043: [RFC] Adding time-trace to LLD?

2019-12-02 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. I've re-implemented this using thread local time tracing. Have broken the changes down into a couple of patches. First one doing some minor tidying up here (https://reviews.llvm.org/D70904). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

2019-11-27 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D69825#1760400 , @aganea wrote: > In D69825#1760373 , @russell.gallop > wrote: > > > It looks like the git apply didn't work, but the script continued so this > > was a duff

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

2019-11-26 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D69825#1760279 , @russell.gallop wrote: > So in this case it saved 0.5% of time. Using the previous maths, with 2378 > clang-cl jobs, this implies process creation time of 29ms. This was fairly > soon after a reboot.

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

2019-11-26 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D69825#1758949 , @aganea wrote: > Thanks for the feedback Russell! > > Can you possibly try again with `/MT`? (ie. `-DLLVM_USE_CRT_RELEASE=MT`) I tried adding this to my stage 1 builds and it didn't make any

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

2019-11-22 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. Thanks for this change. I applied this patch (prior to splitting out https://reviews.llvm.org/D70568) and it built and worked okay (I only see one clang-cl in process explorer). I don't see anything like same performance improvement however. I did my own

[PATCH] D69043: [RFC] Adding time-trace to LLD?

2019-10-18 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D69043#1714060 , @ruiu wrote: > I applied this patch and built clang with ThinLTO. Here is the generated file: > This file seems much smaller than yours. What am I missing? I have seen some very empty looking traces

[PATCH] D69043: [RFC] Adding time-trace to LLD?

2019-10-17 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D69043#1712542 , @ruiu wrote: > This seems useful. Can I see an example output? Thanks. Here's an example from Building clang with ThinLTO: F10296420: clang-10.json Using linker

[PATCH] D69043: [RFC] Adding time-trace to LLD?

2019-10-16 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added reviewers: ruiu, pcc, anton-afanasyev. Herald added subscribers: cfe-commits, dexonsmith, MaskRay, hiraditya, arichardson, mehdi_amini, emaste. Herald added a reviewer: espindola. Herald added projects: clang, LLVM. The clang

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

2019-10-10 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. This is failing the sanitizer lint check: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/23819/steps/64-bit%20check-sanitizer/logs/stdio /compiler-rt/lib/ubsan/ubsan_checks.inc:22: Lines should be <= 80 characters long [whitespace/line_length]

[PATCH] D68710: Remove time-trace message as it is inconsistent style

2019-10-10 Thread Russell Gallop via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. russell.gallop marked an inline comment as done. Closed by commit rG9d9ac46a08d7: Remove rest of time-trace message as it is inconsistent style (authored by russell.gallop). Changed prior to commit:

[PATCH] D68710: Remove time-trace message as it is inconsistent style

2019-10-10 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 224278. russell.gallop added a comment. Update so --help helps user find trace file. Documentation has the further details on how to view it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68710/new/ https://reviews.llvm.org/D68710 Files:

[PATCH] D68710: Remove time-trace message as it is inconsistent style

2019-10-10 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop marked 2 inline comments as done. russell.gallop added inline comments. Comment at: clang/include/clang/Driver/Options.td:1797 + HelpText<"Turn on time profiler. Generates JSON file based on output filename. " + "Results can be analyzed with

[PATCH] D68710: Remove time-trace message as it inconsistent style

2019-10-09 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added reviewers: anton-afanasyev, sylvestre.ledru. Herald added a project: clang. https://reviews.llvm.org/D68260 removed some of the message that -ftime-trace prints out. Running large builds still prints out a lot of "Time trace json-file

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

2019-09-30 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. > The other thing worth checking is the clang PGO self-host on Windows. > This has the potential to break that, and the fix would be to add a linker > flag in LLVM's cmake. This does indeed break PGO self-host with lld-link (applied on top of r373200):

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

2019-08-19 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop accepted this revision. russell.gallop added a comment. This revision is now accepted and ready to land. I don't know a lot about the structure of clang but it LGTM from the point of view of the code and traces coming out. I'm not very keen on having two "Frontend" sections, but

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

2019-06-24 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added inline comments. Comment at: llvm/lib/Support/TimeProfiler.cpp:67 // Only include sections longer than TimeTraceGranularity msec. -if (duration_cast(E.Duration).count() > TimeTraceGranularity) This comment looks wrong since this

[PATCH] D62850: [X86] Fix builtins-x86.c test where it wasn't using immediates. NFC

2019-06-05 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 203124. russell.gallop added a comment. Re-added test cases using variables and added comment. This now tests both formats. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62850/new/ https://reviews.llvm.org/D62850 Files:

[PATCH] D62850: [X86] Fix builtins-x86.c test where it wasn't using immediates. NFC

2019-06-04 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. > We support non immediate on these because gcc does. Thanks. Your comment crossed in mid-air. Okay, so is this test worth changing, or should it have both versions (immediate and non-immediate)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D62850: [X86] Fix builtins-x86.c test where it wasn't using immediates. NFC

2019-06-04 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. > I'll have a look and see if there is a reason why these don't fail in the > same way (which would make the test fail in it's current form). These do not have the "I" prefix (// I -> Required to constant fold to an integer constant expression.) in

[PATCH] D62850: [X86] Fix builtins-x86.c test where it wasn't using immediates. NFC

2019-06-04 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. > Then the test should be failing? Or is the current form also legal? Hmm, __builtin_ia32_insertps128() errors if you pass a variable, but these don't (e.g.): mytest.c:122:13: error: argument to '__builtin_ia32_insertps128' must be a constant integer tmp_V4f

[PATCH] D62850: [X86] Fix builtins-x86.c test where it wasn't using immediates. NFC

2019-06-04 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. > Is the compiler missing a check that these parameters are immediates? I don't think that there can be a check, or this would have been noticed. I don't know whether this is possible and/or desirable. Do users use one version of the builtin and want the

[PATCH] D62850: [X86] Fix builtins-x86.c test where it wasn't using immediates. NFC

2019-06-04 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. > .. or? I'm not sure what you mean. > Is this fixing a current test failure? No. The test is not failing, but it is not doing what was intended as these builtins are for generating the immediate form of the corresponding instruction and they were generating

[PATCH] D62850: [X86] Fix builtins-x86.c test where it wasn't using immediates. NFC

2019-06-04 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added reviewers: craig.topper, cfe-commits. russell.gallop added a project: clang. Some of these test cases were using a variable instead of a literal so were not generating the immediate form of the corresponding instruction. Repository:

[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-28 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. > I was going to suggest that maybe what we should do is just embed the > basename, i.e. /nodefaultlib:clang_rt.profile-x86_64.lib ... Do you mean /defaultlib:clang_rt.profile-x86_64.lib? > ... and then we just ask users to add one /libpath: flag to their linker

[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-28 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. > it looks like passing -no-canonical-prefixes makes this path relative Do you see this working? I tried writing a test and it doesn't appear to work for me. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61742/new/

[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-28 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a subscriber: ruiu. russell.gallop added a comment. > Hello, this embeds an absolute path into the generated .obj file, which means > the output now is no longer deterministic (since it depends on the absolute > path to clang_rt.profile-x86_64.lib). Yes, it embeds the

[PATCH] D62200: [Driver][Windows] Add dependent lib argument for -fprofile-generate and -fcs-profile-generate

2019-05-21 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added reviewers: rnk, hans, bogner. russell.gallop added a project: clang. Follows on from r360674 which added it for -fprofile-instr-generate. Identified when doing: https://reviews.llvm.org/D62063 Repository: rG LLVM Github Monorepo

[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-10 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. In D61742#1497259 , @rnk wrote: > Thanks, I would like to do this for the sanitizers as well, since this is a > constant pain point for users That would be good. I think that this might already work for UBSan. CHANGES

[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-10 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop updated this revision to Diff 199016. russell.gallop added a comment. Herald added a subscriber: mstorsjo. Prevent adding -dependent-lib on mingw32 and add tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61742/new/ https://reviews.llvm.org/D61742 Files:

[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-09 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added reviewers: rnk, bogner. russell.gallop added a project: clang. This is needed so lld-link can find clang_rt.profile when self hosting on Windows with PGO. Trying to self host on Windows with PGO runs into undefined symbols as lld-link

[PATCH] D61264: Fix inconsistency in calculating DIAG_START values

2019-04-29 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop created this revision. russell.gallop added reviewers: xazax.hun, rsmith. russell.gallop added a project: clang. Herald added a subscriber: rnkovacs. The inconsistency was introduced at r313975. As DIAG_SIZE_CROSSTU and DIAG_SIZE_COMMENT are both 100 this should be NFC.

[PATCH] D43578: -ftime-report switch support in Clang

2018-04-11 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop added a comment. We also see an assertion failure prior to the revert. At r329738: $ cat test.cpp template struct A { template using rebind_alloc = _Other; }; template struct _Wrap_alloc { template using rebind_alloc = typename A<_Alloc>::template