Skip to content

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
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 <[email protected]>
Co-authored-by: Steven Johnson <[email protected]>
Co-authored-by: Andrew Adams <[email protected]>
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.

3 participants