Page MenuHomePhabricator

[GVNSink] Make GVNSink resistant against self referencing instructions (PR36954)
Needs ReviewPublic

Authored by yurai007 on Nov 15 2021, 7:15 AM.



Before this change GVNSink pass suffers from stack overflow while processing self referenced instruction in unreachable basic block.
According [1] and [2] it's reasonable to make pass resistant against self referencing instructions.
To fix issue we skip sinking analysis for block in which such instruction is detected.


Diff Detail

Event Timeline

yurai007 created this revision.Nov 15 2021, 7:15 AM
yurai007 requested review of this revision.Nov 15 2021, 7:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2021, 7:15 AM

Skip analysis for unreachable BBs?

nikic added inline comments.Nov 16 2021, 12:27 PM

This checks for a directly self-referential instruction, but the references may also be cyclic across multiple instructions.

As @xbolva00 mentioned, it may be simpler/cleaner to skip unreachable blocks.


Can this test be further reduced?

yurai007 added inline comments.Nov 17 2021, 2:18 AM

Yes, and that's what I proposed as alternative solution: It would save some work and resolve cycles problem. The only drawback I see now is need of giving GVNSink access to DT since we need isReachableFromEntry or analogous machinery for checking reachability. I started with first, simpler approach because it was preferred by @jeroen.dobbelaere, however checking cycles with this idea during lookupOrAdd run seem to be nontrivial. So yes, I can switch to second approach and check where it leads.

yurai007 updated this revision to Diff 387894.Nov 17 2021, 3:54 AM
yurai007 edited the summary of this revision. (Show Details)
yurai007 marked an inline comment as done.