- User Since
- Jun 10 2016, 1:55 AM (136 w, 5 d)
Wed, Jan 9
Tue, Jan 8
This looks good to me, but I don't think I can give an official L-G-T-M.
Mon, Jan 7
Dec 20 2018
Dec 17 2018
Dec 14 2018
I do not find the test file on trunk. Also, git log -- test/Transforms/Util/dbg-user-of-aext.ll in the git mirror does not yield anything, so it seems like the file has not previously existed either.
With this patch applied we don't see any issues on our end. Thanks for the help!
Dec 11 2018
Dec 10 2018
Thanks for the reviews! I'm planning on submitting this shortly then.
Dec 4 2018
Nov 26 2018
Nov 15 2018
Nov 12 2018
Add another non-metadata user of @global_arr to the test case so that the verification of the global's use-list order fails when running without this patch. Also generalize the comment in the test case.
Nov 8 2018
In the previous revision I had overlooked two things in the writers:
Nov 2 2018
Thanks for your reviews and suggestions! I'm planning on submitting this shortly.
Nov 1 2018
Does anyone more than @vsk have any thoughts on this?
Oct 31 2018
Oct 26 2018
I thought I had built up some confidence for this after a night of fuzz testing. However, first after creating this review I encountered a crash in the LL parser, which happened due to the use-list order containing too many elements with this patch.
Oct 16 2018
Sorry, I can likely not give any valuable input for this review.
Oct 15 2018
Thanks for the reviews! I'm planning on submitting this shortly.
Oct 12 2018
Move DbgValues and the findDbgValues() call closer to the use.
Oct 11 2018
Any thoughts on this?
Oct 1 2018
- Changed so that getCalledFunction() allows null valued operands.
- Rewrote the test case to a unit test.
Sep 27 2018
It would be good to change the title to reflect what is done in MCP (at least before submitting this).
Sep 26 2018
Sep 7 2018
Sep 6 2018
Thanks for the review! I'll submit this tomorrow when I have time to track the build bots.
Sep 5 2018
Removed unnecessary !dbg locations in the test. I also moved the file to test/DebugInfo/.
Sep 4 2018
Jul 6 2018
I don't really know anything about the AMDGPU target, so I'll resign as reviewer to make that clear, but thanks for notifying me about the fix!
Jul 5 2018
Hi! We encountered a UBSan runtime error after this was merged. I wrote a bug report about it: https://bugs.llvm.org/show_bug.cgi?id=38071.
Jul 2 2018
Thanks for the reviews!
Jun 27 2018
Use pop_back_val() for getting the items from the worklist.
Jun 20 2018
May 31 2018
May 30 2018
Any more comments or concerns, or can I land this?
May 29 2018
May 28 2018
Addressed vsapsai's comments.
Update patch to include the clang-tools-extra test case originally added in D47251.
May 25 2018
Query TheDriver.isSaveTempsEnabled() at uses instead of storing the value in the constructor.
Renamed SaveTempsEnabled field to KeepTempFiles.
I have now updated the patch so that the files are removed when deleting Compilation objects.
May 24 2018
Thanks for the review! I'll submit this tomorrow when I have time for the build bots.
May 23 2018
We have added a lit reproducer for this now in clang-tools-extra: https://reviews.llvm.org/D47251.
May 22 2018
May 18 2018
Thanks for the reviews! I'll submit this shortly.
May 17 2018
Any more comments on this?
May 10 2018
May 9 2018
@bruno, @vsapsai: I added you since you I saw that you recently reviewed, respectively delivered, D30881. That is the only DependencyFile commit since October; although, this feels a bit orthogonal from that, so feel free to remove yourselves as reviewers (and I'm sorry for adding you in that case)!
Apr 25 2018
Ping. It feels a bit nasty that the tools leave behind temporary files, so I think that it would be good to find a fix for that.
Apr 18 2018
Apr 16 2018
Apr 11 2018
Hi! We have encountered a regression where clang-tidy leaves behind temporary files after this change. I wrote a PR for that: https://bugs.llvm.org/show_bug.cgi?id=37091.
Mar 28 2018
Our legacy frontend does not support -MJ, so when using that frontend for code generation, we also invoke clang with -MJ, and at the same use -fsyntax-only to get the improved diagnostics that clang provides. This is idiosyncratic and probably hacky, I know, but it works well enough to for example for getting access to defines and include flags from the compilation database, and being able to run clang-tidy. So (1) does not fit our use case, unfortunately.
With this change, we will emit the .file 0 directive even for -gdwarf-. The directive results in an error when assembling with GAS. Should this be seen as a problem, or is it something that we accept?
Mar 26 2018
Downstream we use -MJ in a bit of an idiosyncratic way, as we're in a transition period where we, for a subset of the code base, only use the clang frontend for diagnostics, and not for the code generation. However, if you don't think that using -fsyntax-only and -MJ makes sense in any upstream application, I'll drop from this change. I'm leaving the assertion as-is.