- User Since
- Sep 8 2014, 5:28 AM (319 w, 21 h)
Jan 17 2017
Jan 11 2017
Jan 9 2017
Nov 25 2016
Nov 22 2016
LGTM. I have one question/suggestion inline.
Thanks for reviewing this! I committed this in r287646.
Nov 20 2016
Nov 17 2016
Nov 15 2016
Nov 2 2016
LGTM but there's a wrong constant that you should change before committing this.
Oct 27 2016
Oct 25 2016
I didn't consider all the cases but it seems that the macro expansion is probably fine in terms of correctness. However, the expansion is different from that of GAS' and this will generate different object files unnecessarily. As far as I can tell, GAS handles these macro in 3 different ways depending on the value of the immediate:
Oct 24 2016
Oct 21 2016
Oct 19 2016
We generate different code from the GNU assembler which in some cases is wrong, see inline comment.
LGTM once its dependency lands.
Why shouldn't we "crash" when we pass invalid/bad constant immediates to our intrinsics? Do you have a case in mind that causes us problems if we do so?
Oct 18 2016
Marking this with "Request Changes" based on my last comment.
When I tested this I noticed that MultiSource/Benchmarks/7zip/7zip-benchmark was failing. I'm marking this with "Request Changes" to better keep track of the pending review requests.
LGTM. See inline comments for a few small changes.
Sep 28 2016
Cleanup FastISel support checks and remember to reject it on MIPS16 as well.
Sep 27 2016
I'm getting a few test failures from the LLVM test-suite for MIPS32R2. Can you take a look at what's going wrong and update the patch before I review it? Given the symmetry of the code I'd expect, more or less, the same test failures to happen for microMIPS32R2 too but I'm not sure.
Hi Sean, is your intention to LGTM this patch by accepting it, or to convey that it's working fine for *BSDs?
Sep 26 2016
We should split this in two separate review requests, ie. one that introduces symbol dumping and another that marks local and weak symbols. Can you also add a separate test file just for checking the newly introduced -asm-show-symbol-table option?
Sep 22 2016
Abandoning this because arc didn't add llvm-commits.
Sep 21 2016
Sep 20 2016
@sdardis: can you update the review request before committing this because I don't think that test/CodeGen/builtins-mips-msa.c has ever been properly reviewed before.
I removed myself from the reviewers because I'm not quite familiar with this area. @joerg I added you as reviewer because you've already commented, feel free to undo my change if you don't have time to review this.
LGTM with a few minor changes, see inline comments.
Can you take a look at CodeGen/Mips/cconv/arguments-varargs.ll because it fails for me (and rebase the patch while at it :))?
LGTM with one comment/question inline.
Sep 7 2016
LGTM but make sure that the checks from the new test enforce the correct ordering of loads/stores.
Sep 6 2016
Sep 5 2016
When building with BUILD_SHARED_LIBRARIES, such tests don't manage
to find their libraries and are consequently not run (due to a bug
in LIT, this isn't even reported!).
Sep 2 2016
Hi Sebastian. I tried this on one of our BB that tests recursive builds and test/Transforms/GVN/pr28626.ll is failing. I'm getting the same failure on an X86_64 host.
Sep 1 2016
Aug 30 2016
The code changes LGTM but the test case doesn't look like it is testing the right thing.
Aug 17 2016
LGTM with two small test fixes.
Aug 8 2016
Jun 22 2016
Jun 18 2016
Jun 16 2016
Jun 15 2016
Jun 8 2016
Jun 5 2016
May 23 2016
From what I remember it boils down to the fact that we don't want to have atomics whose width isn't natively supported on our targets. Take a look at the comments from this thread: http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160125/328321.html.
May 16 2016
May 13 2016
Apr 22 2016
LGTM with the addition of one test for the M3 prefix.
Apr 20 2016
Hi Nitesh, you can go ahead and commit this. But you should change the comments from:
Apr 17 2016
LGTM. I have just one question inline.