- User Since
- Oct 19 2012, 12:57 AM (431 w, 5 d)
This is the error:
Mon, Jan 25
Sun, Jan 24
Fri, Jan 22
Overall, I think this is great. It answers all questions I've seen posed on the RFCs and is well aligned with the tiers policy, in text and intent.
Wed, Jan 20
Tue, Jan 19
Sat, Jan 16
Fri, Jan 15
Wed, Jan 13
Also, please use diff -U9999 as documented: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
Wed, Jan 6
Tue, Jan 5
Thu, Dec 31
Wed, Dec 30
Tue, Dec 29
Dec 21 2020
Dec 18 2020
This LGTM now, thanks!
LGTM too, thanks!
Dec 17 2020
Dec 16 2020
Looks like a standard mock implementation, basic infrastructure. I don't see anything wrong with it but I've also been far from this area for too long, so I'll let other people review it properly and approve.
Dec 10 2020
LGTM too, thanks!
This looks good to me but I'll let other people review again and approve. Thanks!
@craig.topper are you happy with the changes?
Dec 9 2020
Dec 4 2020
Dec 2 2020
Thanks for the changes. This looks good to me but I'll let others check again and approve.
Nov 30 2020
I can't possibly comment on the ISA implementation, but this looks like a standard table-gen set of files to me. They also don't affect anything else outside of CSKY, so LGTM.
Nov 19 2020
You should at least get @craig.topper's feedback, given this is a significant change in the x86 backend.
Nov 18 2020
Nov 17 2020
Nice set of tests, well separated, not a lot of pattern-match on the FileCheck side, which is good, asm and object emission.
I only had a very quick overview and this looks fine.
A few nits, but this is looking good.
Nov 13 2020
Hi @fpetrogalli, the document is so dense that it took me a while to check the macros and I was still wrong.
Nov 11 2020
Nov 10 2020
It's looking a bit redundant to me, but I wanted to make sure we have the link on all three sections in case people are given a link (or navigate from the table-of contents) and don't see the sections above.
Nov 7 2020
Sorry, forgot the review text on the commit message:
Closed by 25ba6b2bcd1
Thanks Chris! Will do.
- Include a short phrase on inclusion policy, as requested.
- Fixed some typos
Nov 6 2020
Simple and straightforward, with documentation! :)
Nice change, simpler to read and easier to get right. LGTM, thanks!
Sorry, s/George/Geoffrey/. :S
- Adding inclusion policy section
- Reword phrase in deprecation policy
Nov 5 2020
Rewriting some confusing paragraphs.
Updating simple suggestions / fixups.
Nov 4 2020
- Adding page link to "Getting Involved" page.
- Fixing some formatting errors (Sphinx reports no warnings on my box).
Nov 2 2020
The change looks good to me but @gkistanova usually handles buildbot operations, so I don't want to approve before she has time to look at it.
Oct 29 2020
W00t! FreeBSD FTW!
Oct 28 2020
Accepting again. I particularly like @gribozavr2's suggestion. LGTM, thanks!
Oct 27 2020
I have read all of the comments in this review and have looked at all the other pending reviews because of this and I still see strong disagreement on the design and implementation.
Oct 26 2020
Oct 25 2020
I don't want to "take sides" and I'm only looking at this review from what it is and my own experience in the community. But looking at D83088, I see a somewhat problematic review.
Oct 24 2020
Oct 23 2020
Oct 16 2020
Oct 8 2020
@MaskRay are you happy with the latest changes? If so, please change your review. Thanks!
Oct 1 2020
Adding @craig.topper as the owner of the x86 back-end, to check the implementation of the CodeBeads concept. I have mainly worked with RISC back-ends, so it's not my area. Craig, please feel free to add whoever you think is best to review this.
There are plenty of lint checks that need addressing. Some are false positives and those are obvious cases, but the rest should be followed.
Sep 25 2020
No problems, feel free to close, thanks!