- User Since
- Jun 28 2018, 11:39 AM (81 w, 1 d)
Mon, Jan 13
Tue, Jan 7
I left similar comments on D72357 too
I left comments on D72357 instead
Thu, Jan 2
Fri, Dec 20
Thanks -- the code in question seems to be heavily templated, so it may be a corner case with how those are handled. I'll revert shortly and follow up with some kind of repro, but may be delayed due to holidays.
Thu, Dec 19
We're seeing a large memory increase in compilations as a result of this patch: 4.7G -> 6.0G (25%)
Dec 18 2019
Dec 17 2019
If other reviewers agree, then let's just remove the warning. I can send a patch tomorrow unless someone else wants to do that.
It seems the discussion of whether or not this is incomplete died out -- I'd prefer to assume it is incomplete if there is no consensus. Mailed D71635 to rename -frounding-math to -fexperimental-rounding-math.
Looks good for D/U, but looks like --help and --version options are also supported as combined short args; do you mind adding those too while you're here?
Dec 13 2019
Dec 12 2019
Dec 11 2019
Dec 10 2019
Looks good. Thanks for adding the tests!
lit substitutes llvm-ar with the full path; see llvm/test/tools/llvm-ar/case-detection.test
Ditto; my direct goal is just to be able to run something more fine grained than ninja check-lldb-api, but splitting tests & test framework is a nice side benefit. (Actually it might be simpler if I didn't split it, but now I've already done the work).
Update the heuristic to make all the following work as intended:
This was manually verified, but can you add a test for it?
You could probably write a lit test like:
Dec 6 2019
Since the diff is large, I'll paste the only non-mechanical part here:
Officially abandoning this
Dec 5 2019
Nov 22 2019
-> D70137 is the correct patch that actually adds this test -- this is testonly cleanup
Nov 20 2019
Nov 19 2019
Nov 15 2019
Nov 14 2019
- Fix test comment
- Use expect helper from lldbpexpect
Nov 12 2019
- Add a pexpect test
Oct 31 2019
I found a strange sanitizer error as a result of this revision, could you take a quick look at https://bugs.llvm.org/show_bug.cgi?id=43870?
It looks like this caused a very large increase in binary size (627M->686M). Is that expected/has anyone else observed this?
Oct 25 2019
LGTM with the wildcard changes, thanks!
LGTM to the change, though I want to resolve this thread before committing:
Not really opposed to this patch, but I think ruiu's comment makes sense to think about:
Oct 24 2019
I'm good with the approach in this patch -- once the test/docs changes are done, I'll accept it. I also ran into Jake yesterday at the llvm devmtg and came to the same conclusion. (Also, sorry for the delay, which was also because of the devmtg). --strip-all should generally be more aggressive, but not so much that it totally doesn't work as a default mode on common platforms.
Oct 21 2019
Oct 18 2019
Also, a couple docs that should be updated (where we mention .gnu.warning):
I agree, we generally want --strip-all to be the aggressive option and --strip-all-gnu to be the option when more compatibility is needed (and --keep-section when things seriously go wrong), but I think we should probably not have --strip-all be completely unusable for default modes.
Oct 17 2019
Generally LGTM, although I think Ray is a much better person to approve this.