- User Since
- Jul 4 2018, 10:36 AM (132 w, 4 d)
Tue, Jan 12
Thu, Jan 7
Removed the addition of isIntS32Immediate() - it is no longer needed after addressing the comment of using APInt.
- Updated names of the selection functions in the td patterns
- Address the comment of using APInt when computing address flags
- Removed NFC from the title as there is one test case update that we expect a DSForm in (instead of an XForm instruction)
Wed, Jan 6
Thanks for updating. I just noticed one more minor nit.
Mon, Jan 4
Dec 17 2020
LGTM. Thanks for the update Albion.
I just have a minor nit, but overall it looks good to me.
I'm really sorry for the delay. LGTM to me, as well.
Dec 16 2020
Add more comments to functions, make provablyDisjointOr() static, fix pattern in PPCInstr64Bit.td.
@steven.zhang The idea is that this patch should be NFC. All existing load/store test cases should pass with this refactoring. I do think there should be more tests added, perhaps in a follow up patch. What do you think?
Dec 15 2020
Please address the comments regarding the test cases.
Dec 4 2020
Just a minor comment but LGTM overall.
Dec 3 2020
Nov 12 2020
Minor nit but LGTM
Nov 11 2020
Please also address the clang-format comment.
Nov 10 2020
Aside from the clang-format, LGTM.
Thanks for fixing for tests and formatting. LGTM if there are no other concerns.
Nov 4 2020
Overall LGTM, thanks for addressing comments Baptiste!
Nov 2 2020
Thanks Baptiste. LGTM.
Oct 28 2020
Oct 27 2020
Aside from the clang-format issues and as long as no one has any other concerns, this LGTM.
Oct 26 2020
Oct 23 2020
Thanks for investigating this Hubert. Overall makes sense and LGTM, as long as there are no other concerns from anyone else.
Oct 22 2020
Address the following review comments:
- updated RUN and CHECK lines
Oct 20 2020
Agreed that a test should be added.
Oct 19 2020
I think it overall LGTM.
Oct 15 2020
Oct 13 2020
- Update the patch with the latest master.
- Remove isMulhCheaperThanMulShift TLI Hook from all code segments.
- Update test/CodeGen/X86/pmulh.ll.
Oct 1 2020
Overall I think this LGTM.
Sep 30 2020
Sep 29 2020
Sep 26 2020
Sep 24 2020
So, my goal was to introduce a DAG combine to combine multiply+shifts into mulh. This is done in a function I introduced within DAGCombiner.cpp (called combineShiftToMULH).
Sep 23 2020
- Updated the isEltOfVectorTy() to the correct semantics; making it take in a vector type and then getting the element type within the function.
Sep 22 2020
@nemanjai Would you please take another look to see if I have addressed your comments when you get a chance? Thanks.
Sep 21 2020
Address Nemanja's review comments:
- More specific comments when bitcasting the inputs
- Pull out conditions to bitcast the input, use ternary op depending if the input is 32 or 64-bits
- Create new static function to check if a given type is the same type as a vector element
Updated identation/formatting of code in CGExprScalar.cpp and altivec.h.
Thanks for open coding. Aside from Nemanja's nits, I believe all my concerns have been addressed. LGTM.
Sep 18 2020
Aside from the clang-format issues that can be addressed on commit this looks good to me.
Overall LGTM as well.
Forgot to approve this patch.
Address clang-format concerns.
Please address clang-format issues and Lei's comments. Other than that, I think the patch LGTM.
Sep 17 2020
- Rebased patch.
- Update patch to remove unnecessary immediate handling.
Sep 16 2020
Sep 15 2020
A question I have is, is it possible for the 128-bit vector modulo instructions be open coded?
Update the run line of crbits.ll test.
Sep 14 2020
@nemanjai Could you please take another look to see if I have addressed your comments?
Sep 6 2020
Sep 3 2020