hans (Hans Wennborg)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 18 2013, 6:48 AM (291 w, 6 d)

Recent Activity

Tue, Nov 20

hans accepted D54723: [COFF] Add exported functions to gfids table for /guard:cf.

lgtm

Tue, Nov 20, 12:52 AM

Thu, Nov 15

hans accepted D54540: [ADT] Drop llvm::Optional clang-specific optmization for trivially copyable types.
Thu, Nov 15, 1:19 AM

Tue, Nov 13

hans committed rC346748: UserManual: Tweak the /Zc:dllexportInlines- docs some.
UserManual: Tweak the /Zc:dllexportInlines- docs some
Tue, Nov 13, 1:08 AM
hans committed rL346748: UserManual: Tweak the /Zc:dllexportInlines- docs some.
UserManual: Tweak the /Zc:dllexportInlines- docs some
Tue, Nov 13, 1:08 AM
hans added a comment to D54319: clang-cl: Add documentation for /Zc:dllexportInlines-.

Thanks for the review! I made adjustments in r346748.

Tue, Nov 13, 1:07 AM

Mon, Nov 12

hans accepted D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag.

lgtm

Mon, Nov 12, 7:26 AM
hans added inline comments to D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag.
Mon, Nov 12, 6:41 AM
hans committed rL346647: Win snapshots: restore the key id.
Win snapshots: restore the key id
Mon, Nov 12, 3:50 AM
hans committed rL346646: Win snapshot: r346630.
Win snapshot: r346630
Mon, Nov 12, 3:50 AM
hans committed rC346640: Release notes: Mention clang-cl's /Zc:dllexportInlines- flag.
Release notes: Mention clang-cl's /Zc:dllexportInlines- flag
Mon, Nov 12, 12:45 AM
hans committed rL346640: Release notes: Mention clang-cl's /Zc:dllexportInlines- flag.
Release notes: Mention clang-cl's /Zc:dllexportInlines- flag
Mon, Nov 12, 12:45 AM
hans committed rC346639: clang-cl: Add documentation for /Zc:dllexportInlines-.
clang-cl: Add documentation for /Zc:dllexportInlines-
Mon, Nov 12, 12:40 AM
hans committed rL346639: clang-cl: Add documentation for /Zc:dllexportInlines-.
clang-cl: Add documentation for /Zc:dllexportInlines-
Mon, Nov 12, 12:40 AM
hans closed D54319: clang-cl: Add documentation for /Zc:dllexportInlines-.
Mon, Nov 12, 12:40 AM

Fri, Nov 9

hans created D54319: clang-cl: Add documentation for /Zc:dllexportInlines-.
Fri, Nov 9, 8:14 AM
hans accepted D54298: [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback.

lgtm

Fri, Nov 9, 4:59 AM
hans added inline comments to D54298: [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback.
Fri, Nov 9, 2:24 AM

Thu, Nov 8

hans committed rL346393: clang-cl: Add "/clang:" pass-through arg support..
clang-cl: Add "/clang:" pass-through arg support.
Thu, Nov 8, 3:29 AM
hans committed rC346393: clang-cl: Add "/clang:" pass-through arg support..
clang-cl: Add "/clang:" pass-through arg support.
Thu, Nov 8, 3:29 AM
hans closed D53457: clang-cl: Add "/clang:" pass-through arg support..
Thu, Nov 8, 3:29 AM
hans added a comment to D53457: clang-cl: Add "/clang:" pass-through arg support..

Reid, Hans, or someone else with commit access. If the revision looks good, could you please submit to SVN?

Any particular testing I should run beforehand? I ran the clang tests locally on Windows.

Thu, Nov 8, 3:17 AM

Wed, Nov 7

hans added a comment to D54171: [MS] Zero out ECX in __cpuid in intrin.h.

Thanks for fixing!

Wed, Nov 7, 1:22 AM
hans added a comment to D53467: Branch/tag all projects with a single commit in release-tagging script..

Still lgtm.

Wed, Nov 7, 1:18 AM

Tue, Nov 6

hans accepted D53457: clang-cl: Add "/clang:" pass-through arg support..

Okay, looks good to me (with a small nit).

Tue, Nov 6, 5:31 AM

Mon, Nov 5

hans committed rL346122: Exclude wasm target from Windows packaging due to PR39448.
Exclude wasm target from Windows packaging due to PR39448
Mon, Nov 5, 1:34 AM

Fri, Nov 2

hans accepted D51340: Add /Zc:DllexportInlines option to clang-cl.

Assuming the test still passes when you put the EXPORTINLINE filecheck prefix back, looks good to me!

Fri, Nov 2, 3:39 AM
hans added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Thanks! I don't think the new isThisDeclarationADefinition() and isImplicitlyInstantiable() checks are right. Besides that, I only have a few more comments about the test.

Fri, Nov 2, 1:54 AM

Thu, Nov 1

hans added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Thu, Nov 1, 7:37 AM
hans added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Thank you Takuto! I think the functionality looks great now: it's not too complicated :-)

Thu, Nov 1, 1:24 AM

Wed, Oct 31

hans added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Wed, Oct 31, 6:30 AM
hans added a comment to D53457: clang-cl: Add "/clang:" pass-through arg support..

The -Xclang option has the same issue, and I think /clang: should work similarly, i.e. /clang:-MF /clang:<filename>. It's not pretty, but at least it's consistent. Yes, that means processing consecutive /Xclang: options together, but hopefully that's doable.

It looks like the handling for -Xclang is a lot simpler (in Clang::ConstructJob). There the Xclang options are all gathered and forwarded at a particular spot in the command line for cc1. They aren't interleaved with other options, and it wouldn't really make sense to do so anyway since it doesn't really look like cc1 arguments are constructed from driver arguments in any particular order.

The llvm/lib/Option/OptTable.cpp code is not really setup to easily interleave arguments like would be required for your request. Can you see a way to accomplish what you want without a deep refactoring of OptTable::ParseArgs and the InputArgList class?

Wed, Oct 31, 3:53 AM
hans committed rL345709: Follow-up to r345699: Call CheckStaticLocalForDllExport later for templates.
Follow-up to r345699: Call CheckStaticLocalForDllExport later for templates
Wed, Oct 31, 3:37 AM
hans committed rC345709: Follow-up to r345699: Call CheckStaticLocalForDllExport later for templates.
Follow-up to r345699: Call CheckStaticLocalForDllExport later for templates
Wed, Oct 31, 3:37 AM
hans added a comment to D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496).

r345709 should do it.

Wed, Oct 31, 3:37 AM
hans added a comment to D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496).

Bah, I should have known something was going to break if I touched this :-)
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/37132
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/1017

Wed, Oct 31, 3:11 AM
hans added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Wed, Oct 31, 2:53 AM
hans added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Wed, Oct 31, 1:52 AM
hans added inline comments to D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496).
Wed, Oct 31, 1:50 AM
hans committed rL345699: [clang-cl] Inherit dllexport to static locals also in template instantiations….
[clang-cl] Inherit dllexport to static locals also in template instantiations…
Wed, Oct 31, 1:41 AM
hans committed rC345699: [clang-cl] Inherit dllexport to static locals also in template instantiations….
[clang-cl] Inherit dllexport to static locals also in template instantiations…
Wed, Oct 31, 1:41 AM
hans closed D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496).
Wed, Oct 31, 1:41 AM

Tue, Oct 30

hans created D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496).
Tue, Oct 30, 8:08 AM
hans added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.
$ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o lib.so lib.cc
$ g++ main.cc lib.so
/tmp/cc557J3i.o: In function `S::bar()':
main.cc:(.text._ZN1S3barEv[_ZN1S3barEv]+0xd): undefined reference to `foo()'

So this is a known issue with -fvisibility-inlines-hidden. This doesn't come up often in practice, but when it does the developer needs to deal with it.

Yeah, that is the reason of few chromium code changes.
https://chromium-review.googlesource.com/c/chromium/src/+/1212379

Tue, Oct 30, 2:30 AM

Mon, Oct 29

hans added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Mon, Oct 29, 7:32 AM
hans added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

I've been thinking more about your example with static locals in lambda and how this works with regular dllexport.

Mon, Oct 29, 7:18 AM
hans added a comment to D53457: clang-cl: Add "/clang:" pass-through arg support..
One note about flag ordering: the /clang: flags are concatenated to the end of
the argument list, so in cases where the last flag wins, the /clang: flags
will be chosen regardless of their order relative to other flags on the driver
command line.

This seems a little unfortunate. I wonder if it would be possible to not have this restriction, i.e. to process these in-line with the rest of the flags?

One way to achieve this would be to change Driver::ParseArgStrings() to handle the "/clang:" arguments. After doing the regular option parsing, it would look for "/clang:" options and substitute them for the underlying option. This has the downside of adding some option-specific logic to ParseArgStrings, but on the other hand it's less intrusive than passing around the IsCLMode bool. Do you think something like this could work?

One difficulty occurs with options that have separated values, like the /clang:-MF /clang:<filename> case. In this case, we need to take two /clang: arguments and process them next to each other in order to form a single coherent argument/value pair. Another option is to allow the user to specify the option like /clang:"-MF <filename>" and then require that the user specify any flags that need a value in a single /clang: argument. This would require some code to split the value string on spaces before passing it on to clang argument parsing.

Do you have any preference or better suggestion for the option-with-value case?

Mon, Oct 29, 3:11 AM

Fri, Oct 26

hans committed rL345389: Win snapshot: r345380..
Win snapshot: r345380.
Fri, Oct 26, 7:54 AM
hans added a comment to D53457: clang-cl: Add "/clang:" pass-through arg support..
One note about flag ordering: the /clang: flags are concatenated to the end of
the argument list, so in cases where the last flag wins, the /clang: flags
will be chosen regardless of their order relative to other flags on the driver
command line.
Fri, Oct 26, 6:37 AM
hans committed rC345380: Revert r345330 "Add MS ABI mangling for operator<=>.".
Revert r345330 "Add MS ABI mangling for operator<=>."
Fri, Oct 26, 6:08 AM
hans committed rL345380: Revert r345330 "Add MS ABI mangling for operator<=>.".
Revert r345330 "Add MS ABI mangling for operator<=>."
Fri, Oct 26, 6:08 AM
hans accepted D53598: Add docs+a script for building clang/LLVM with PGO.

lgtm

Fri, Oct 26, 2:23 AM

Thu, Oct 25

hans accepted D53617: [AArch64] Support Windows stack probe command-line arguments..

lgtm

Thu, Oct 25, 7:59 AM
hans added a comment to D53598: Add docs+a script for building clang/LLVM with PGO.

I didn't have time to look at the script yet, but I read through the instructions doc. For most users I think maybe that's also the most important part, because I think many folks build quite differently. I will try to get to reading the script tomorrow.

Thu, Oct 25, 7:58 AM

Tue, Oct 23

hans committed rL345026: Revert r345009 "[DebugInfo] Generate debug information for labels. (After fix….
Revert r345009 "[DebugInfo] Generate debug information for labels. (After fix…
Tue, Oct 23, 6:19 AM
hans committed rC345026: Revert r345009 "[DebugInfo] Generate debug information for labels. (After fix….
Revert r345009 "[DebugInfo] Generate debug information for labels. (After fix…
Tue, Oct 23, 6:19 AM
hans added inline comments to D52707: Switch optimization in IR for known maximum switch values.
Tue, Oct 23, 5:20 AM · Restricted Project
hans added a comment to D52002: Switch optimization for known maximum switch values.

Anyone have any thoughts about potential security implications for this? Normally, I wouldn't really care about assuming code never has undefined behavior, but an indirect jump to an arbitrary address is much easier to exploit than other sorts of undefined behavior.

Tue, Oct 23, 5:11 AM
hans added a comment to D53457: clang-cl: Add "/clang:" pass-through arg support..

I'm not completely convinced that we want this. So far we've used the strategy of promoting clang options that are also useful in clang-cl to core options, and if someone wants to use more clang than that, maybe clang-cl isn't the right driver for that use-case.

But I suppose an argument could be made for having an escape hatch from clang-cl if it doesn't add too much complexity to the code.

This is a good point. However, having this escape hatch gets you and Reid and others out of the business of having to promote options. Also, as new flags are added to the compiler people might need one revision of the official builds to realize they need a flag and one revision to get the flag added to the binary release. This obviously isn't a problem for people building Clang from source, but it does add unnecessary friction as I found myself.

Tue, Oct 23, 5:03 AM
hans accepted D53476: [clang-cl] Allowed -fopenmp work and link openmp library from per-runtime library directory.

Looks good to me.

Tue, Oct 23, 4:53 AM
hans added inline comments to D52707: Switch optimization in IR for known maximum switch values.
Tue, Oct 23, 4:49 AM · Restricted Project
hans added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Hans, I addressed all your comments.
How do you think about current implementation?

Tue, Oct 23, 4:45 AM

Oct 19 2018

hans accepted D53467: Branch/tag all projects with a single commit in release-tagging script..

Looks good to me, thanks!

Oct 19 2018, 7:58 PM
hans added a comment to D53457: clang-cl: Add "/clang:" pass-through arg support..

I haven't started looking at the code yet.

Oct 19 2018, 4:31 PM
hans committed rC344789: Java annotation declaration being handled correctly.
Java annotation declaration being handled correctly
Oct 19 2018, 9:22 AM
hans committed rL344789: Java annotation declaration being handled correctly.
Java annotation declaration being handled correctly
Oct 19 2018, 9:22 AM
hans closed D53434: Java annotation declaration being handled correctly.
Oct 19 2018, 9:22 AM

Oct 15 2018

hans committed rL344517: 7.0.0: Add link to the ppc binary.
7.0.0: Add link to the ppc binary
Oct 15 2018, 7:46 AM

Oct 13 2018

hans committed rC344469: Try harder to fix test/Driver/cl-showfilenames.c.
Try harder to fix test/Driver/cl-showfilenames.c
Oct 13 2018, 3:25 PM
hans committed rL344469: Try harder to fix test/Driver/cl-showfilenames.c.
Try harder to fix test/Driver/cl-showfilenames.c
Oct 13 2018, 3:25 PM
hans committed rC344462: Re-commit r344234 "clang-cl: Add /showFilenames option (PR31957)".
Re-commit r344234 "clang-cl: Add /showFilenames option (PR31957)"
Oct 13 2018, 12:16 PM
hans committed rL344462: Re-commit r344234 "clang-cl: Add /showFilenames option (PR31957)".
Re-commit r344234 "clang-cl: Add /showFilenames option (PR31957)"
Oct 13 2018, 12:16 PM

Oct 11 2018

hans added a comment to rL344234: clang-cl: Add /showFilenames option (PR31957).

Sorry for the breakage.

Oct 11 2018, 12:25 PM
hans added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Oct 11 2018, 8:26 AM
hans added a comment to D53103: Support for the mno-tls-direct-seg-refs flag.

Looks good to me. Craig should probably sign off too though.

Oct 11 2018, 7:21 AM
hans added a comment to D53102: Support for the mno-tls-direct-seg-refs flag.

Looking good, just a few minor comments.

Oct 11 2018, 7:15 AM
hans added a comment to D52002: Switch optimization for known maximum switch values.

This is looking pretty good, so I'd suggest start writing tests. For testing, take a look at test/CodeGen/X86/switch.ll

Oct 11 2018, 7:06 AM
hans added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Oct 11 2018, 6:58 AM
hans committed rC344234: clang-cl: Add /showFilenames option (PR31957).
clang-cl: Add /showFilenames option (PR31957)
Oct 11 2018, 3:06 AM
hans committed rL344234: clang-cl: Add /showFilenames option (PR31957).
clang-cl: Add /showFilenames option (PR31957)
Oct 11 2018, 3:06 AM
hans closed D52773: clang-cl: Add /showFilenames option (PR31957).
Oct 11 2018, 3:06 AM
hans added inline comments to D52773: clang-cl: Add /showFilenames option (PR31957).
Oct 11 2018, 2:57 AM
hans added a comment to D53052: [AST] Use -fvisibility value when ignoring -fv-i-h* inline static locals.
In D53052#1261158, @rnk wrote:

This is rewritten enough that you might want to re-review it, but I'm going to push it today so we don't have to wait another day/night cycle for the fix.

Oct 11 2018, 1:01 AM

Oct 10 2018

hans added a comment to D52707: Switch optimization in IR for known maximum switch values.

Thanks for doing this! I have some comments, and as was pointed out, this needs tests.

Oct 10 2018, 7:27 AM · Restricted Project
hans added a comment to D52002: Switch optimization for known maximum switch values.

Sorry for the slow response.

Oct 10 2018, 7:08 AM
hans added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Thanks! I think this is getting close now.

Oct 10 2018, 6:59 AM
hans updated the diff for D52773: clang-cl: Add /showFilenames option (PR31957).

Here's a version now using cc1 flags, but printing directly from the driver. Please take a look.

Oct 10 2018, 6:50 AM
hans accepted D53052: [AST] Use -fvisibility value when ignoring -fv-i-h* inline static locals.

Most codebases probably only use either -fvisibility=hidden or
-fvisibility-inlines-hidden, since marking inlines hidden should be a
no-op if everything was already going to be hidden, but it looks like
Chromium uses both.

Oct 10 2018, 1:09 AM

Oct 9 2018

hans added inline comments to D51340: Add /Zc:DllexportInlines option to clang-cl.
Oct 9 2018, 7:44 AM

Oct 5 2018

hans added a comment to D51340: Add /Zc:DllexportInlines option to clang-cl.

Ping?

Oct 5 2018, 1:06 AM

Oct 4 2018

hans added a comment to D52773: clang-cl: Add /showFilenames option (PR31957).

Canyou add a test that demonstrates that multiple input files on the command line will print all of them?

Oct 4 2018, 5:13 AM

Oct 3 2018

hans added a comment to D52798: [cl-compat] Change /JMC from unsupported to ignored..

Thanks for fixing! Please also update the test in test/Driver/cl-options.c and include me on clang-cl code reviews.

Oct 3 2018, 2:21 AM

Oct 2 2018

hans added inline comments to D52773: clang-cl: Add /showFilenames option (PR31957).
Oct 2 2018, 6:19 AM
hans created D52773: clang-cl: Add /showFilenames option (PR31957).
Oct 2 2018, 2:09 AM

Oct 1 2018

hans committed rL343458: Revert r343407 "[InstCombine] try to convert vector insert+extract to trunc".
Revert r343407 "[InstCombine] try to convert vector insert+extract to trunc"
Oct 1 2018, 5:09 AM

Sep 28 2018

hans committed rCRT343322: Revert r342652 "[winasan] Unpoison the stack in NtTerminateThread".
Revert r342652 "[winasan] Unpoison the stack in NtTerminateThread"
Sep 28 2018, 7:43 AM
hans committed rL343322: Revert r342652 "[winasan] Unpoison the stack in NtTerminateThread".
Revert r342652 "[winasan] Unpoison the stack in NtTerminateThread"
Sep 28 2018, 7:43 AM
hans added a comment to D52091: [winasan] Unpoison the stack in NtTerminateThread.

This appears to have caused the thread exit code to get clobbered, as shown by some Chromium test failures: https://bugs.chromium.org/p/chromium/issues/detail?id=890310

Sep 28 2018, 7:43 AM
hans added a reviewer for D52613: [ADT] Change the `IntervalMap` alignment assert for x86 MSVC: rnk.

Hmm, it's strange that we haven't hit this assert before, then. What is causing you to hit it?

Sep 28 2018, 5:16 AM

Sep 27 2018

hans committed rL343206: arm and aarch64 binaries for 7.0.0 and 6.0.1.
arm and aarch64 binaries for 7.0.0 and 6.0.1
Sep 27 2018, 6:04 AM
hans committed rL343189: Revert r342942 "[MachineCopyPropagation] Reimplement CopyTracker in terms of….
Revert r342942 "[MachineCopyPropagation] Reimplement CopyTracker in terms of…
Sep 27 2018, 3:05 AM