Page MenuHomePhabricator

[IRSim] Ignore Debug instructions with constructing canonical numbering for basic blocks
ClosedPublic

Authored by AndrewLitteken on Apr 16 2022, 2:17 PM.

Details

Summary

When constructing canonical relationships between two regions, the first instruction of a basic block from the first region is used to find the corresponding basic block from the second region. However, debug instructions are not included in similarity matching, and therefore do not have a canonical numbering. This patch makes sure to ignore the debug instructions when finding the first instruction in a basic block.

Diff Detail

Event Timeline

AndrewLitteken created this revision.Apr 16 2022, 2:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2022, 2:17 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
AndrewLitteken requested review of this revision.Apr 16 2022, 2:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2022, 2:17 PM
bjope added a comment.Apr 17 2022, 2:22 AM

Thanks for the patch. I've verified that it solves the problem I found.

No comments about the patch itself really. Looks nice, but I don't know that much about this analysis pass to say if it is correct to skip dbg intrinsics like this. (And should for example lifetime instrinsics be skipped in the same way?)

I don't know that much about this analysis pass to say if it is correct to skip dbg intrinsics like this. (And should for example lifetime instrinsics be skipped in the same way?)

Lifetime intrinsics are currently excluded from being matched for similarity, but that's mostly because it causes problems for the outliner. Debug instructions are allowed to be in an outlined region, but are not used in the similarity matching itself. This is mostly because the IRSimilarityIdentifier was built for the outliner, but there has been some discussion about separating the two further and building a specific Identifier for the outliner itself and then one for general similarity. This is (yet another) piece that might push that decision.

paquette accepted this revision.Apr 18 2022, 10:36 AM

LGTM

I think that debug info handling might be a good argument for splitting the similarity identifier into a similarity finding version + outlining version. The outliner needs to be considerate about what it does to debug info, but the similarity identifier doesn't. Someone very well may want to ask questions about debug info similarity in the future.

This revision is now accepted and ready to land.Apr 18 2022, 10:36 AM
This revision was landed with ongoing or failed builds.Apr 19 2022, 11:18 AM
This revision was automatically updated to reflect the committed changes.