Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[SLP][NFC] Cleanup: Add while loop around for loop instead of modifying iterators.
Needs ReviewPublic

Authored by vporpo on May 2 2023, 6:57 PM.

Details

Reviewers
ABataev
RKSimon
Summary

Updating the begin/end iterators of the for loop within its body is a bit confusing.
Wrapping it with a while loop makes it explicit that we may iterate more than once.

Diff Detail

Event Timeline

vporpo created this revision.May 2 2023, 6:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 2 2023, 6:57 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
vporpo requested review of this revision.May 2 2023, 6:57 PM

I assume the process is restarted not to try to redo the analysis (it looks like useless) but to fix possible issue with the deleted/modified instructions. Could you try to use for (Instruction &I : make_early_inc_range(BB)) instead and avoid outer loop?

I assume the process is restarted not to try to redo the analysis (it looks like useless) but to fix possible issue with the deleted/modified instructions. Could you try to use for (Instruction &I : make_early_inc_range(BB)) instead and avoid outer loop?

Yeah that may be the issue, but I am getting test failures with it. Could it be that we need to revisit already vectorized instructions?

I assume the process is restarted not to try to redo the analysis (it looks like useless) but to fix possible issue with the deleted/modified instructions. Could you try to use for (Instruction &I : make_early_inc_range(BB)) instead and avoid outer loop?

Yeah that may be the issue, but I am getting test failures with it. Could it be that we need to revisit already vectorized instructions?

That's possible, if we emit extractelements. Could you try to do it in reverse order, something like make_early_inc_range(reverse(BB))?