[clang] [CIR][AArch64] Lower NEON vbsl builtins (PR #188449)
Andrzej Warzyński via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 6 08:22:04 PDT 2026
https://github.com/banach-space approved this pull request.
This is quite polished, well done and thank you for contributing! **LGTM % minor comments.**
While preparing this to be merged, could you update the summary to focus on "what" and "why"? Specifically, I am suggesting not to use `castVecOfFPTypeToVecOfIntWithSameWidth` from the incubator, so you will have to update the corresponding part.
Also, while the following comments are helpful during the review and the discussion, I would not include them in the summary:
```
(...)
If this is not the preferred way to structure it, I’d be happy to adjust it based on your feedback.
(...)
Update 4,1,2026:
```
The summary should _only_ cover "what" and "why" - that would be more consistent with how we approach summaries in LLVM. You can post the other comments (which are helpful) as "regular" comments in your PR (here is an example from one of my PRs: https://github.com/llvm/llvm-project/pull/181357#issuecomment-4073800542).
Writing good summaries is hard and takes practice. This blog post should be quite helpful:
* https://shafik.github.io/software%20development/2025/09/22/why-summaries-are-important-for-prs.html
Here's another great post on Git commit messages (within LLVM, "GitHub summary" == "Git commit message"):
* https://chris.beams.io/git-commit
In fact, that blog post is recommended in MLIR docs:
* https://mlir.llvm.org/getting_started/Contributing/#commit-messages
Thank you again for contributing 🙏🏻
https://github.com/llvm/llvm-project/pull/188449
More information about the cfe-commits
mailing list