- User Since
- Jul 4 2018, 10:36 AM (119 w, 5 d)
Agreed that a test should be added.
I think it overall LGTM.
Thu, Oct 15
Tue, Oct 13
- Update the patch with the latest master.
- Remove isMulhCheaperThanMulShift TLI Hook from all code segments.
- Update test/CodeGen/X86/pmulh.ll.
Thu, Oct 1
Overall I think this LGTM.
Wed, Sep 30
Tue, Sep 29
Sat, Sep 26
Thu, Sep 24
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).
Wed, Sep 23
- Updated the isEltOfVectorTy() to the correct semantics; making it take in a vector type and then getting the element type within the function.
Tue, Sep 22
@nemanjai Would you please take another look to see if I have addressed your comments when you get a chance? Thanks.
Mon, Sep 21
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
Sep 1 2020
Aug 28 2020
Aug 26 2020
Aug 24 2020
Update to address clang-format.
Update clang test names for vec_expandm.
Aug 21 2020
Address review comments:
- Further consolidate the custom codegen of the two builtins
- Add SemaChecking for if the third argument is a constant, if the third argument is in range and if the second argument is the same type as the element type of the first argument
- Add extra test to test the semantic checks that were added
Aug 19 2020
Aug 17 2020
Aug 11 2020
Aug 10 2020
Aug 9 2020
Thanks for addressing the comments. LGTM.
Aug 7 2020
I realized I didn't put a comment on this earlier but this overall LGTM, but I think it would be good to see if @nemanjai has any additional comments on this patch.
Aug 6 2020
Aug 5 2020
Aug 4 2020
Rebased patch, and removed MC tests from the original patch.
Rebased patch and removed MC tests from the patch.
Rebased the patch and removed MC tests.
I think overall it LGTM and the indentation can be addressed when committing.
Jul 31 2020
Jul 27 2020
Could we also elaborate in the description on how we are utilizing the new load instructions for zero extend case but not the sign extend case?
Thanks for recovering the tests. LGTM.
- Rebased patch
- Updated patch to remove instruction definitions and MC tests
Addressed the following comments:
- updated CHECK lines to check for the full intrinsic call
- updated indentation