- User Since
- Apr 6 2019, 11:05 AM (52 w, 3 d)
Fri, Apr 3
(But still deferring approval to somebody with more experience.)
Thu, Apr 2
In the spirit of https://llvm.org/docs/CodeReview.html#non-experts-should-review-code, I am reviewing the code. But I defer the approval of this patch to somebody else who has more experience than me. (I found only a single nit.)
Tue, Mar 31
Mon, Mar 30
- Updated ExtractCast.ll test.
Let me look at the failed test in a couple of hours.
- Updated the test.
- Changed uint32_ts to uint64_t and added an assert to catch overflows.
Update the new trunc-extractelement.ll test file.
Sun, Mar 29
The new preliminary tests have been submitted in https://reviews.llvm.org/D77024. I removed the test for now and will submit another update when the other diff has been committed.
My git lingo might be off here. Are you saying you don't have commit permissions for LLVM yet?
@spatel Thanks for the review. I will soon look into it!
Sat, Mar 28
I didn't look at the logic closely, but seems to be on the right track from the tests (feel free to include Alive2 links in the review if you tested any/all of these).
I see at least 2 variations where we need more code logic (and more tests):
Do not canonicalize if the extractvector operand has other users.
Do not canonicalize if it would result in an invalid bitcast instruction.
It was a bit unclear to me how involved the tests should be for this patch. At this time, I kept them pretty minimal, but I could add more if that's desired.
Dec 17 2019
Awesome. Thanks for reviewing!
Nov 5 2019
I don't have commit right. This is one of my first patches. Can you do it for me please?
Oct 29 2019
Fixed formatting using clang-format.
@jhenderson, you were right. I forgot how to upload patches correctly, and that is how the context got lost. The new diff (226891) should be exported correctly. Thanks for the heads-up.
Updated the diff to one generated in the correct way.
Updated the diff incorporating the new comments.
Updated the diff, incorporating @hintonda's suggestions.
Oct 28 2019
@jhenderson, I added the test.
Added OptionErrorMessage test case in CommandLineTest for PR42943.
Jun 8 2019
Jun 2 2019
Removed the pattern for RORRd.
Jun 1 2019
May 19 2019
- Use the temporary registers r1 and r0, instead of scavenging spare registers.
- And update the test case.
- Add the definition const unsigned ZERO_REGISTER = AVR::R1 for future use.
May 16 2019
Thank you for the feedback. I will try to update the patch code this weekend.