- User Since
- Jun 20 2019, 2:39 AM (232 w, 3 d)
Jul 17 2023
I'm not currently the right person to review this, resigning.
May 19 2023
First off, thanks for the discourse!
May 18 2023
Sure, simply run the readelf commands from this commit message or something similar on the clang toolchain you built, both in your build and install directories, and perhaps with this commit reverted too.
Looks like my previous message was truncated. I think it is 'not incorrect' for the rpath to point to LLVM_LIBRARY_DIR, which is the install path at least for the nixpkgs llvm install. In this case, the prefix clang is installed to differs from the one LLVM is installed to. If the rpath isn't set for clang there, it can't find libLLVM.
If any LLVM subprojects are built separately, the LLVM build directory LLVM_LIBRARY_DIR is added to both the build and install runpaths in llvm_setup_rpath(), which is incorrect when installed.
Apr 17 2023
Mar 7 2023
Makes sense to me, one minor suggestion. I note that this introduces a few loads/stores to the stack for fixed length vectorization, but having it work correctly is the priority, those may be possible to remove later.
Feb 27 2023
Are there other parent revisions? The base isn't a valid commit and the patch doesn't apply to main nor commits from the end of January, for example CompareByComesBefore isn't in the patch context but is on main and is old.
Jan 25 2023
Jan 24 2023
I've been down the list of MCA output changes and agree those look good, and your reasoning seems good to me. Accepting as-is, since we think represents an improvement.
Jan 23 2023
Jan 18 2023
Dec 21 2022
Wrong action selected... :)
Thanks for the input Florian!
Dec 20 2022
- Fix test naming.
- Also cover the zero passthru case.
Address review comments:
- Remove superflous braces.
- Prefer indexed operations in the presence of a non-undef passthrough to a dup.
Dec 14 2022
Dec 13 2022
LGTM, but please address the comment about poison and allow time for other reviewers to chime in.
Dec 12 2022
- Update a test name (an old one crept in)
- Remove 'q' accidentally copied into the test names.
- Test zext/sext.
- Test unpacked forms (no negative tests, since dup with unpacked types is not specifically supported currently as there is currently no known way to get a DUP_MERGE_PASSTHRU from C/C++ code).
- Drop immediate tests since those are covered.
Dec 8 2022
- Rebase after b4028fbc1a88
- Address review comments: Move comment down and rename BisectExtendVector => HalveAndExtendVector.
Dec 7 2022
Thanks Bradley -- I'll continue this one in the near future though your input is welcomed.
Nov 28 2022
LGTM with minor comments.
Nov 23 2022
Nov 21 2022
Nov 14 2022
LGTM, Thanks for trying the path of making it generic and thanks for the input from the other reviewers!
Nov 10 2022
One suggestion inline.
Nov 8 2022
Nov 7 2022
LGTM, but adding other reviewers for visibility. The predictable ask is to make it a target-agnostic combine: reviewers, thoughts?
Nov 3 2022
Nov 2 2022
This is my first gambit, and I'm relatively new to making changes to LoopVectorize. Please don't hold back if this is the wrong approach, I'm happy to put legwork in to finding the right route to making this work.
Nov 1 2022
LGTM, with one minor nit.
Oct 24 2022
LGTM, one nit.
LGTM. (With the fix) I've tested this produces the expected output.
Oct 18 2022
Looks good to me, with one minor suggestion.
Oct 11 2022
(Full disclosure: Ben is a new colleague of mine)
Sep 22 2022
Sep 8 2022
Aug 15 2022
Aug 11 2022
I will submit first thing next week unless there are further comments.
- Fix copy-paste error for the legality check for the ZERO_EXTEND case.
Ihanks @RKSimon. The tests are unaffected. I've opted to follow your suggestion.
Aug 10 2022
Aug 9 2022
Thanks for the review comments, Simon!
New patch at https://reviews.llvm.org/D131503
Aug 8 2022
Jul 28 2022
- Stylistic changes to shrink the code a little.
- Add check for signext_inreg type.
- Update tests.