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

[CodeGen] Fix InstructionCount remarks for MI bundles #107621

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

francisvm
Copy link
Collaborator

For MI bundles, the instruction count remark doesn't count the instructions inside the bundle.

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

This is probably by design. Code is supposed to logically treat a bundle as one instruction

@francisvm
Copy link
Collaborator Author

This is probably by design. Code is supposed to logically treat a bundle as one instruction

Some targets use bundles to mark instruction packet boundaries, no? IMO, that shouldn't be counted as one instruction.

@arsenm
Copy link
Contributor

arsenm commented Sep 9, 2024

This is probably by design. Code is supposed to logically treat a bundle as one instruction

Some targets use bundles to mark instruction packet boundaries, no? IMO, that shouldn't be counted as one instruction.

Yes, that was the original use (assuming some details about how packets are represented). I don't know what "InstructionCount" here is supposed to really mean though

@francisvm
Copy link
Collaborator Author

This is probably by design. Code is supposed to logically treat a bundle as one instruction

Some targets use bundles to mark instruction packet boundaries, no? IMO, that shouldn't be counted as one instruction.

Yes, that was the original use (assuming some details about how packets are represented). I don't know what "InstructionCount" here is supposed to really mean though

I think regardless of bundles, they're still real MachineInstrs that get emitted, I don't see why they wouldn't get counted as regular instructions. I can see this count being a problem for pseudos that get expanded directly into MCInsts, but not for bundles.

@francisvm francisvm force-pushed the instruction-count-remarks-bundle branch from 6ba6dff to 3483c7c Compare September 9, 2024 18:20
@francisvm francisvm force-pushed the instruction-count-remarks-bundle branch from 3483c7c to 7be8d66 Compare September 27, 2024 01:04
@francisvm francisvm force-pushed the instruction-count-remarks-bundle branch 3 times, most recently from 218a464 to 3caf9d4 Compare October 2, 2024 01:05
@francisvm francisvm force-pushed the instruction-count-remarks-bundle branch 3 times, most recently from 70e5a57 to 53d83c0 Compare October 2, 2024 14:48
For MI bundles, the instruction count remark doesn't count the
instructions inside the bundle.
@francisvm francisvm force-pushed the instruction-count-remarks-bundle branch from 53d83c0 to fdffa1a Compare October 2, 2024 17:37
@francisvm francisvm merged commit 916e6ad into llvm:main Oct 2, 2024
8 checks passed
@francisvm francisvm deleted the instruction-count-remarks-bundle branch October 2, 2024 20:37
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024
For MI bundles, the instruction count remark doesn't count the
instructions inside the bundle.
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
For MI bundles, the instruction count remark doesn't count the
instructions inside the bundle.
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.

2 participants