Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for fcvtm/p, make scalars go through pattern matching too #8151

Merged
merged 1 commit into from
Mar 13, 2024

Conversation

abadams
Copy link
Member

@abadams abadams commented Mar 12, 2024

This change fixes the only regression from main I could find in the sve branch, and improves things on top of that.

@abadams abadams requested a review from zvookin March 12, 2024 19:46
Copy link
Member

@zvookin zvookin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me. I guess SVE expects one to do the integer conversion using frint with an appropriate rounding mode first.

@abadams abadams requested a review from halidebuildbots March 12, 2024 23:55
@abadams
Copy link
Member Author

abadams commented Mar 13, 2024

Looks like the bots will only test PRs to main. I guess I'll just merge it and fix anything it breaks in your branch.

@abadams abadams merged commit 9d8e2c6 into arm_sve_redux Mar 13, 2024
3 checks passed
abadams added a commit that referenced this pull request Mar 15, 2024
* Checkpoint SVE2 restart.

* Remove dead code. Add new test.

* Update cmake for new file.

* Checkpoint progress on SVE2.

* Checkpoint ARM SVE2 support. Passes correctness_simd_op_check_sve2 test at 128 and 256 bits.

* Remove an opportunity for RISC V codegen to change due to SVE2 support.

* Ensure SVE intrinsics get vscale vectors and non-SVE ones get fixed vectors.

Use proper prefix for neon intrinsics.

Comment cleanups.

* Checkpoint SVE2 work. Generally passes test, though using both NEON
and SVE2 with simd_op_check_sve2 fails as both posibilities need to be
allowed for 128-bit or smaller operations.

* Remove an unfavored implementation possibility.

* Fix opcode recognition in test to handle some cases that show up.

Change name of test class to avoid confusion.

* Formatting fixes.

Replace internal_error with nop return for CodeGen_LLVM::match_vector_type_scalable called on scalar.

* Formatting fix.

* Limit SVE2 test to LLVM 19.

Remove dead code.

* Fix a degenerate case asking for zero sized vectors via a HAlide type
with lanes of zero, which is not correct.

* Fix confusion about Neon64/Neon128 and make it clear this is just the
width multiplier applied to intrinsics.

* REmove extraneous commented out line.

* Address some review feedback. Mostly comment fixes.

* Fix missed conflict resolution.

* Fix some TODOs in SVE code. Move utility function to Util.h and common
code the other obvious use.

* Formatting.

* Add missed refactor change.

* Add issue to TODO comment.

* Remove TODOs that don't seem necessary.

* Add issue for TODO.

* Add issue for TODO.

* Remove dubious looking FP to int code that was ifdef'ed out. Doesn't
look like a TODO is needed anymore.

* Add issues for TODOs.

* Update simd_op_check_sve2.cpp

* Make a deep copy of each piece of test IR so that we can parallelize

* Fix two clang-tidy warnings

* Remove try/catch block from simd-op-check-sve2

* Don't try to run SVE2 code if vector_bits doesn't match host.

* Add support for fcvtm/p, make scalars go through pattern matching too (#8151)

* Don't do arm neon instruction selection on scalars

This revealed a bug. FindIntrinsics was not enabled for scalars anyway,
so it was semi-pointless.

---------

Co-authored-by: Zalman Stern <zalman@macbook-pro.lan>
Co-authored-by: Steven Johnson <srj@google.com>
Co-authored-by: Andrew Adams <andrew.b.adams@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants