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

[AArch64][GlobalISel] Add legalization for G_VECREDUCE_MUL #68398

Merged
merged 3 commits into from
Oct 16, 2023

Conversation

chuongg3
Copy link
Contributor

@chuongg3 chuongg3 commented Oct 6, 2023

No description provided.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@chuongg3 chuongg3 force-pushed the GlobalISel_VECREDUCE_MUL branch from 1e6e59f to c75c915 Compare October 6, 2023 10:00
@tschuett
Copy link

tschuett commented Oct 6, 2023

Good style: state legal types before clamping and give a mir test as you touched the legaliser.

Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

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

Looks good. Can you rebase to try and move away from the failing BOLT tests? Thanks

; RUN: llc -mtriple=aarch64 -verify-machineinstrs %s -o - 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-SD
; RUN: llc -mtriple=aarch64 -global-isel -global-isel-abort=2 -verify-machineinstrs %s -o - 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-GI

; CHECK_GI: warning: Instruction selection used fallback path for mulv_v3i62
Copy link
Collaborator

Choose a reason for hiding this comment

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

62->64

@@ -885,6 +885,14 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
.clampMaxNumElements(1, s16, 8)
.lower();

getActionDefinitionsBuilder(G_VECREDUCE_MUL)
.clampMaxNumElements(1, s64, 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

For i64 a max clamp of 1 is the same as scalarizing, and this line could be omitted.

@chuongg3 chuongg3 force-pushed the GlobalISel_VECREDUCE_MUL branch from 91452b5 to 47d44c9 Compare October 10, 2023 12:48
Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

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

Thanks, I think this LGTM

@chuongg3 chuongg3 force-pushed the GlobalISel_VECREDUCE_MUL branch from 47d44c9 to ace4b3d Compare October 16, 2023 08:43
@chuongg3 chuongg3 merged commit dad563e into llvm:main Oct 16, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants