- User Since
- Jan 10 2013, 2:43 PM (297 w, 21 h)
Wed, Sep 19
Since it helps existing msbuild configs, adding this seems like a good thing to me.
FWIW the recommendation against /Ox in my version is because of https://github.com/ulfjack/ryu/pull/70#issuecomment-412168459
Mon, Sep 17
Thanks! Changed the comment; landing.
Sat, Sep 15
fix case on variable
The revert helped, see e.g. http://lab.llvm.org:8011/builders/lld-x86_64-darwin13/builds/25686 and http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/23378 (compare to previous build)
I reverted this (and follow-on fix attempts r342154 r342180 r342182 r342193) in r342336 in an attempt to heal the lld bots.
Ah, looks like all changes are mentioned at the top, just not as comments further down.
https://reviews.llvm.org/rLLD342334 too. Thanks!
Fri, Sep 14
rebase, minor cleanups
Unrelatedly, another problem with the current design is that llvm.srcdir.txt must contain an absolute path, which makes it impossible to build on one machine and then copy artifacts and test inputs to another machine and run tests on another machine. DebugInfoPDBTests is the only test in all llvm, lld, clang tests that has this restriction. Requiring some defined fixed pwd for unit tests with inputs would address this issue as well. (As would finding a way to not require reading a file off disk for this test.)
Thu, Sep 13
Actually, this replaces https://reviews.llvm.org/D51887
This replaces https://reviews.llvm.org/D51957.
The situation on Windows is like on Linux. I built chrome.dll with our pinned lld (which was compiled with clang), with a locally-built lld (built by msvc 2017), and then with this patch and the other two patches linked from here. Min-of-5 times are comparable (full data in parens):
Wed, Sep 12
(Forcing an email to be sent since I accidentally clicked "save" before adding llvm-commits)
Tue, Sep 11
Did some measurements on my linux box (didn't have access to my win box). There, just naively calling xxHash64 on the 1.4GB chrome.dll.pdb file after creating it, doing the parallel xxHas64 computation, this patch, and the old nondeterministic guid code all take about the same amount of time (around 32s, min-of-4 links with each approach is within 0.2s of that). I'll try this on Windows tomorrow.
(Landed r341945 to address the 2 differing bytes that caused the guid for rsds.test to be different. It's still different due to different /pdbaltpath: flags, but the pdb contents are now identical except for guid and timestamp. I don't understand yet how /pdbaltpath: makes it into the hash, but for the same /pdbaltpath: it seems to be deterministic now. Looking more…)
The new test fails like so on my mac laptop:
Mon, Sep 10
Sat, Sep 8
Fri, Sep 7
Thu, Sep 6
We may call these "unit" tests, but they're really gtests, and developers everywhere use gtest for more than just unit tests. We already have tests that create temp files. I don't see why reaching back into the source directory for inputs is too heavyweight for us.
Also also, opening a file from a unit test kind of makes it not a unit test, since those are supposed to not hit the disk etc. Maybe that test wants to be its own binary instead that's called from a lit script, instead of being a unit test?
Sorry, didn't see this earlier. Writing a llvm.srcdir.txt next to every single unit test binary seems like a pretty roundabout way of doing this. How about instead passing the src dir in a runtime flag in http://llvm-cs.pcc.me.uk/utils/lit/lit/formats/googletest.py#107 if the lit config asks for it? Then you don't need to touch the disk to get the src dir path, it's not passed to _all_ binaries, and cmake doesn't have to write a file to disk for every test binary.
What's the status here? Did this land?
Wed, Sep 5
Tue, Sep 4
Quoting from the PE spec (see bug for more): "The raw data of this debug entry may be empty". So I don't think it was a bug. Maybe tools had problems with the zero-sized entries, but they didn't bother to update the spec. So I'd suggest to go with this and only add more code if needed (?). We could add a size and just add the 4-byte hash to TimeDateStamps to fill it in at the end, but while it's easy to do it feels a bit cargo-culty.
MSVC on my laptop has the same behavior as on my desktop:
pcc: what if you do cl /Zi /Brepro foo.c?
The test fails on my system like so:
Mon, Sep 3
r341313, thanks! I had svn mvd the files. Diffs for mv'd files look a bit weird; phab is probably doing the best it can do here.
Sun, Sep 2
We don't match gcc's -Wextra behvior. We generally try to not put a ton of stuff in Wextra that isn't in -Wall. (We also generally don't put a lot of stuff in -Wall that isn't enabled by default.) So I don't think we want this.
Thu, Aug 30
r341130 (clang-tools-extra), r341132 (clang), r341134 (lld), and r341135 (llvm). Thanks!
Tue, Aug 28
Mon, Aug 27
Thu, Aug 23
FWIW I'd like to use the new demangler in demumble , but if I need to pull in most of Support of it then I probably won't do that. I agree this shouldn't impact LLVM much (or at all), but having stand-alone-ish demangler code is probably nice for several other projects too. So since the itanium demangler is still in the other place and likely will stay there, maybe it's nice to keep the ms demangler next to it. (But as I said, don't weigh this feedback heavily.)
Wed, Aug 22
Thanks, that's a good suggestion. I put the new function in Common/Args.h; I'm not sure if that's the best place.
Aug 22 2018
Aug 21 2018
Aug 20 2018
Aug 17 2018
Can you explicitly mention that this intentionally doesn't use an absolute path in MicrosoftMangleContextImpl() or similar?
Forgot to add cfe-commits :-( Doing that now.