- User Since
- Mar 12 2014, 3:00 PM (236 w, 1 d)
Thu, Aug 23
Aug 19 2018
Aug 16 2018
Aug 15 2018
Aug 13 2018
LGTM as long as Hal's good with your explanation of why we can't use TinyPtrVector.
The cmake and lit bits all look correct
Aug 10 2018
What exactly are out of tree targets supposed to migrate to?
Aug 7 2018
Aug 3 2018
Aug 1 2018
Jul 26 2018
I believe this fixes the issues I was hitting.
Jul 24 2018
Seems straightforward and correct to me.
Jul 23 2018
Jul 19 2018
Conceptually this looks pretty good, but we should definitely sink a lot of this implementation into lib/Support/FileCheck.cpp and anything that stays in the header should be moved into the llvm:: namespace, since this is library code now. There are a lot of overly generic names in the implementation as is (like Pattern), which is fine as long as they end up in anonymous namespaces in FileCheck.cpp but will have to be changed if they're really part of the API.
Jul 10 2018
Committed in r336708.
Jul 9 2018
Update to set path to codesign_allocate correctly and set the extra tool paths in platforms/iOS.cmake.
Jul 3 2018
This fixes the segfault / asan error I was seeing in ADTTests with -Os on macOS, and the trade off with the runtime check seems pretty reasonable to me.
Jun 29 2018
Jun 28 2018
Jun 27 2018
Jun 26 2018
Jun 14 2018
Jun 13 2018
Why? (As in, please update the commit message to explain why)
A couple of very minor nitpicks about use of auto, below, but overall this looks very nice. LGTM.
Jun 11 2018
Jun 7 2018
This makes a lot of sense, especially given that the change to use SparseBitVector here instead of std::set specifically called out that this was to improve cases where there were dense register files. LGTM.
May 23 2018
May 10 2018
May 4 2018
May 3 2018
Looks good, but please use update_llc_test_checks for the existing tests and move the test with the explanation to its own file
May 2 2018
Apr 30 2018
Apr 26 2018
Apr 10 2018
Would this be easier to follow if we overloaded printMemOperand with a variant that took the variables we're declaring here (M, MF, etc) and simply called into it? The "if (G)" condition would set them up and the "else" side would just pass null pointers.
Apr 4 2018
Okay. lg as an incremental improvement.
Apr 3 2018
Can we do better than just calling these namedVRegN? That is, is there a scheme where we could give these more meaningful names that would still be relatively stable between different input IRs?
Mar 30 2018
Mar 21 2018
Still looks good - two very nitpicky style comments below for when you commit.
Mar 14 2018
Nice improvement! I have a couple of minor style/clarity comments, but LGTM with those addressed.
Mar 13 2018
Mar 12 2018
Mar 8 2018
I've commented on both parts, but please keep each review to a single topic. Adding the test is one thing, and adding the helpers should be another review (or better yet, just add them when you use them).
Seems straightforward enough. LGTM.
Mar 7 2018
Mar 2 2018
One last comment, then I think this is good to go.
Feb 28 2018
Looks good, thanks
Feb 27 2018
Okay, I guess this makes sense for now. I think we should name the flag -disable-mir-legality-check instead of -disable-legality-check, as the current name is a bit too general sounding.
Feb 22 2018
Looks good, pending what we decide to do about the testing.
I like the idea of adding tests for update_mir_test_checks itself, but I think we should stick to focused tests rather than just adding one that tries to test everything. I also think this isn't a great place for it, maybe we could add a test/tools/update_checks folder for tests for all of the update_*_tests scripts? We might need to fake out llc to get away with something like that though...
Feb 21 2018
Feb 16 2018
Sure. I don't love the translations to underscores (it makes it a bit harder to map things) but I don't have a better idea offhand. Since this is starting to move towards "every pass" we should maybe consider doing something less hand rolled soon.
Feb 9 2018
Feb 8 2018
Feb 7 2018
Seems reasonable. Adding a couple of folks who know a bit more about debug info than I do to approve this.
Feb 5 2018
This seems like a bit of a hack compared to actually fixing the TODO and moving this check into the MachineVerifier where it belongs.
Seems reasonable. A couple of nitpicks.
Feb 2 2018
I think this is okay to go in, with the caveat that we need a plan for fixing up the verifier and removing this.
Jan 30 2018
I have mixed feelings about this, but I guess it's probably better than the status quo. My two main concerns are the time cost and whether we'll stop noticing the issues instead of fixing them.
Jan 27 2018
Jan 26 2018
This is looking pretty good. Let's get it in tree so people can start using it and we can iterate more based on feedback.
Jan 24 2018
I find the naming conventions for the matchers kind of weird, but I see that they're meant to be familiar to users of the IR level matcher APIs so I suppose they at least make sense. Let's get this in and start using it.