- User Since
- Feb 16 2014, 1:10 AM (257 w, 5 h)
Thu, Jan 10
Great, thanks for the review!
Wed, Jan 9
Remove obsoleted code I accidentally included.
Thanks for the offline review @GorNishanov! This revision allows constexpr usages of __builtin_coro_frame_max_size.
Sun, Jan 6
Thank you for the review!
Thu, Jan 3
Wed, Jan 2
Fri, Dec 21
Excellent progress! Thanks for working on this.
Dec 19 2018
Dec 18 2018
Dec 17 2018
Dec 12 2018
Dec 11 2018
LGTM! As it happens I was reading this code the other day and wondering why it wasn't able to eliminate a suspend point I'd been looking at. I'll try it again and see if this new behavior is able to eliminate -- thanks!
Dec 10 2018
Sorry for the wait! Mostly nits, but also I think the while condition might be inverted? It seems like, as is, it would never execute the body of the loop...?
Dec 9 2018
Thanks for the review, @reames! I implemented each of your suggestions, but I had a small excuse for not using the variable name Suffix.
I see, thanks @arsenm! I think this variable fits those criteria so I changed it back to using a dollar-sign prefix. Variable names notwithstanding, does the implementation change seem OK to everyone? It fixes what I believe is clearly a programming error. Landing this change will unblock me from LLVM_NODISCARD to these functions and so preventing another case of this same error.
Search the bugpoint directory first, and only fall back to PATH if the program isn't there.
Thanks @MatzeB! I'll change this to search in the bugpoint directory first, and fall back to the PATH.
Thanks for the review, @nhaehnle! I changed the FileCheck variable NOUNWIND_READONLY to not use a dollar-sign prefix. I'm not sure why the other lines in this test do, but I left them alone for now. Any other suggestions for this, or is it good to go?
Dec 7 2018
Dec 5 2018
Oops, sorry, I didn't check hard enough. There's code for AMDGPU that runs up against this warning, causing some buildbots to fail: http://lab.llvm.org:8011/builders/lld-perf-testsuite/builds/9637/steps/build-bin%2Flld/logs/stdio
Thanks! Yup, I confirmed no new warnings when building check-llvm and check-clang.
Dec 3 2018
Nov 28 2018
Friendly ping! Could someone recommend a reviewer for this? Or is there something wrong with the patch?
Friendly ping! I like this patch because it saves me the keystrokes of typing --opt-command every time I run bugpoint. Please give it a look and let me know if I can improve anything.
Nov 25 2018
Nov 5 2018
Sorry to have let this languish! LGTM.
Nov 3 2018
Oct 30 2018
Oct 29 2018
Here's a compiler explorer link demonstrating that GCC 8.2 and Clang 7.0.0 compile the example code, but Clang trunk emits an error: https://godbolt.org/z/l3baI_
Oct 15 2018
Oh, I'm sorry I let this languish! I'll address your comments later this week, @vsk. Thanks so much for the review!
Oct 7 2018
Sep 26 2018
This is great, thanks! Sorry for letting it languish. I defer to @GorNishanov, but I don't see why this couldn't go in now and if there're any edge cases I'm missing we can address those in another patch. I pulled this down and played around with it, and everything seemed OK to me.
Sep 3 2018
Excellent, I think pushing this along with D50410 revealed the true error, as an MSAN buildbot tells use there's a use of an uninitialized value: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/23176/steps/check-clang%20msan/logs/stdio
Nice! This seems like a plain improvement in terms of debugging, thanks.
Thanks! I'll land D50410, then this, and monitor the buildbots to see what happens. If they fail again I may revert this once again. Thanks again for looking into this, sorry for the delay in landing it.
Aug 28 2018
This looks good to me, but maybe some people who've modified this part of the codebase before could review this as well? @aaron.ballman added a fix-it for angled/quoted strings in rL160406, and more recently @erikjv modified some of this code in rL285057. If anyone could suggest other reviewers that would be great as well!
Aug 15 2018
I don't have access to the PS4 SDK, but this is the most plausible explanation I've seen for why I was experiencing issues on these platforms. Thanks for this!
Jul 14 2018
Yup, LGTM! I'll land this now.
Jul 10 2018
If you don't have commit access, let me know here if you'd like me to commit this on your behalf.
Jul 5 2018
This LGTM but I'll just wait for @GorNishanov to accept the patch, just in case I'm missing something. I'd be happy to commit this for you once Gor accepts! :)
Jul 3 2018
Oops, apologies, I included a line I shouldn't have in the previous diff.
Jul 2 2018
Jun 26 2018
Jun 23 2018
Great, thanks @GorNishanov! I moved the 'can throw' logic into a function called 'memberCallExpressionCanThrow', to convey that some dyn_cast'ing is going on.
Great, thanks for the review! I added a reference to N4499.
Also @GorNishanov I'm curious about your two cents on whether comments like these are valuable. If you think they are I may add a few more with post-commit review.