Skip to content

[CodeGen] TwoAddressInstructionPass: Control NumVisited limit via command line option #100125

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

Closed
wants to merge 1 commit into from

Conversation

AZero13
Copy link
Contributor

@AZero13 AZero13 commented Jul 23, 2024

Pulled out of comment made on #80627 - to simplify further investigation into visit limits.

@AZero13 AZero13 changed the title [TwoAddressInstructionPass] Have visit limit be a command line value [CodeGen] TwoAddressInstructionPass: Control NumVisited limit via command line option Jul 23, 2024
@AZero13
Copy link
Contributor Author

AZero13 commented Jul 23, 2024

@RKSimon Is this good?

// Limit the number of rescheduling visits to dependent instructions.
// FIXME: Arbitrary limit to reduce compile time cost.
static cl::opt<unsigned>
MaxVisits("twoaddr-visit-limit", cl::Hidden, cl::init(10),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we include "reschedule" in the option name? "twoaddr-reschedule-visit-limit"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like I am not the person who should make that decision

Copy link
Collaborator

Choose a reason for hiding this comment

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

twoaddr-reschedule-visit-limit sgtm

@RKSimon RKSimon self-requested a review July 23, 2024 16:20
@AZero13 AZero13 force-pushed the cmd branch 3 times, most recently from 38b4afb to de65a1f Compare July 24, 2024 03:17
Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

Please add test coverage to an existing test file where you add an additional RUN with a different twoaddr-reschedule-visit-limit value

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