- User Since
- Nov 16 2017, 6:22 PM (44 w, 5 d)
Mon, Sep 17
I like this change. I have a few nits in the comments, but this looks good to me, caveat: I am not a tablegen expert.
Thu, Sep 13
Abandoning this change, it has gone stale.
Wed, Sep 12
Tue, Sep 11
- Updated the ResourceCycles class description.
- Moved that class into Support.h.
- Removed an unused + operator.
Mon, Sep 10
Sat, Sep 1
Wed, Aug 29
Abandoning this change, it is outdated.
Tue, Aug 28
Mon, Aug 27
Aug 24 2018
Rebased this diff after reorganizing the llvm-mca directory some.
- Move the Resource classes into their own file.
Aug 23 2018
- Updated per comments from the last spin:
- Moved CodeRegion out of the library.
- Added Views and Stats directories to cleanup the layout (Views and Stats are not part of the library).
Aug 22 2018
Aug 21 2018
LGTM, we chatted about the index calculation offline. I am okay with that instruction age calculation.
Aug 20 2018
Aug 18 2018
- Tossed in an LLVMBuild.txt for the library.
- Added library dependencies to the cmake.
Aug 17 2018
Aug 16 2018
I like this change! Is there ever the case where there would be two different DILocations within a bundle? Perhaps that questions doesn't matter since bundles are treated as a single item.
I made a few comments, all just nits. This is a pretty substantial change in design, but better. LGTM. Thanks for this!
Aug 14 2018
Abandoning in favor of https://reviews.llvm.org/D50561
I don't see anything wrong with this. I'm all for better modularity with the tablgen "function predicates"; LGTM.
Aug 13 2018
- Addressed comments:
- Added braces.
- Removed a stale comment and initializer.
- Using Expected::get in a few places instead of operator* in Pipeline.cpp.
Aug 10 2018
Added Stage::Status type alias to clean up the return value for Stage::execute. The previous iteration of this patch used an Expected<bool>, which I find confusing. This change makes the code more readable.
- RegisterFile::isAvailable no longer fataly fails, instead if an instruction requires more registers than what a register file can hold, a warning is issued and the PRF is increased to hold the number of registers specified by the instruction.
- Unfortunately this requires isAvailable to no longer be const, and that the RegisterMappingTracker to no longer have a const NumPhysRegs field.
- Another option is check the register file lengths per instruction, but outside of isAvailable. As part of some sanity check. However, that will some what duplicate the for-loop in isAvailable.
- I'm going to go through the patch and see if there is any template code I can simplify via using directives, I don't want to increase complexity.
Thanks @andreadb. LGTM.
Aug 9 2018
Thanks for this patch! LGTM.
Aug 8 2018
I really like this change! The result is easier to read.
Aug 7 2018
Aug 3 2018
Jul 31 2018
- Simplify the test case, let the compiler join the literals for us.
- Renamed the test output and test directory to clean up the more specific test example.
Jul 30 2018
- Added a test that creates a directory via __llvm_profile_recursive_mkdir and verifies the mode.
- I intend to only commit one of the two tests, but wanted to provide a more specific/detailed test.
- I don't know if we really want such a specific test, but it's a potential start at one.
- We can definitely ignore windows here (since there is no mode passed to its equivalent to mkdir).
- I don't know if there are other systems we need to ignore, e.g., android?
- I'm still hesitant to adding the more specific test, it feels very os specific.
- I didn't see an alternative to FileCheck's (deprecated) %T, so I wrote a simple 'dirname' routine that is not dependent on libgen.
- I hope that is sufficient for this test case.
Thanks for the comments! I have updated the diff based on that feedback.
Jul 27 2018
Jul 25 2018
@orodley No problem. I don't know if you have commit access (your account looks pretty new), but I'm happy to land the patch for you later if you wish.
Thanks for the patch @orodley. Good catch! LGTM.
Jul 23 2018
Jul 21 2018
Jul 20 2018
- Removed some redundant text from the example.
- Captured full context.
Jul 19 2018
Thanks for the feedback @andreadb ! I've already made the suggested changes and will land that shortly.
Jul 18 2018
Jul 17 2018
I think it makes sense to have a separate patch for checkInvalidRegOperand. I do like that idea in particular, it simplifies reading the tablegen sources.