- User Since
- Oct 19 2012, 12:57 AM (442 w, 5 d)
And this one, too: https://reviews.llvm.org/D95030
This looks good to me, thanks!
There are still some comments on the remaining patches in this set that need addressing before all of them can be committed as a whole.
Tue, Mar 16
Actually, this is mentioned in a different document: https://www.llvm.org/docs/Phabricator.html#committing-a-change
Mar 8 2021
Mar 4 2021
Some comments, mostly on style, but this looks like a fair skeleton lowering to me.
+1 to @myhsu's points, plus a few comments, but otherwise, is looking good.
This looks good to me. @MaskRay any comments?
Feb 16 2021
Feb 15 2021
Feb 4 2021
Feb 3 2021
Feb 2 2021
Jan 30 2021
My reading of the manual says this change is correct. All the exceptions and deprecations in MOVs and LSL seem to not be in ARM mode with the S flag set and without the PC in Rm (source register).
Jan 29 2021
Jan 28 2021
ARS, LSR, ROR have the following warning on the ARM ARM:
Jan 27 2021
Jan 26 2021
This is the error:
Jan 25 2021
Jan 24 2021
Jan 22 2021
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.
Jan 20 2021
Jan 19 2021
Jan 16 2021
Jan 15 2021
Jan 13 2021
Also, please use diff -U9999 as documented: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
Jan 6 2021
Jan 5 2021
Dec 31 2020
Dec 30 2020
Dec 29 2020
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).