Skip to content

Conversation

chuongg3
Copy link

@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

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

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

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.

5 participants