Skip to content

Conversation

ivafanas
Copy link
Contributor

No description provided.

@ivafanas
Copy link
Contributor Author

Inspired by comment: #158265 (comment)

I suggest, if llvm already has MachineLoopUtils.h, it also might have MachineInstrUtils.h :)

Copy link

github-actions bot commented Sep 15, 2025

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

@ivafanas ivafanas force-pushed the dev/extract-copy-paste-on-machine-phi-income-deletion branch 3 times, most recently from 8800fae to 0310776 Compare September 18, 2025 09:45
@ivafanas
Copy link
Contributor Author

ivafanas commented Sep 18, 2025

Hi, @arsenm

Could you please review the PR?

If it is ok, could you please merge it, I do not have commit access to the project.

Comment on lines 23 to 24
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also just put this directly into MachineInstr (or alternatively, into MachineBasicBlock and have it handle all phis in the block)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks!

@ivafanas ivafanas force-pushed the dev/extract-copy-paste-on-machine-phi-income-deletion branch from 0310776 to e1a30e7 Compare September 18, 2025 22:45
Comment on lines 1512 to 1513
Copy link
Contributor

Choose a reason for hiding this comment

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

I meant a method, MachineBasicBlock::removePHIIncomingValuesForPredecessor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see.

Updated.

@ivafanas ivafanas force-pushed the dev/extract-copy-paste-on-machine-phi-income-deletion branch from e1a30e7 to 984ecf3 Compare September 19, 2025 12:18
@ivafanas
Copy link
Contributor Author

Ping.

Any update, please?

I'm fully ok to close MR without merge if it is useless.

@arsenm arsenm merged commit 3e63993 into llvm:main Sep 25, 2025
9 checks passed
@ivafanas
Copy link
Contributor Author

Thank you!

mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
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.

3 participants