This is an archive of the discontinued LLVM Phabricator instance.

[SDAG] Introduce a common MEMBARRIER node [nfc]
ClosedPublic

Authored by reames on Jan 9 2023, 12:30 PM.

Details

Summary

We have multiple targets which have defined custom instructions and sdag nodes to represent a compiler memory barrier. This patch consolidates the sdag node definition into common code.

This is a companion to D92842, but a bit different in focus. This change consolidates the existing sdag node definitions; that patch skipped defining a sdag node by instead going straight to a target node. That patch is also not NFC - as being so is quite hard for commoning up the instruction definitions.

I started with two backends to ensure the new common code was reusable while not having a massive diff. Once this lands, I'll submit a series of NFCs for backends where the changes are obvious, or reviews if more discussion is needed.

Diff Detail

Event Timeline

reames created this revision.Jan 9 2023, 12:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 9 2023, 12:30 PM
reames requested review of this revision.Jan 9 2023, 12:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 9 2023, 12:30 PM

Missing an update to SDNode::getOperationName in SelectionDAGDumper.cpp

reames updated this revision to Diff 487556.Jan 9 2023, 2:26 PM

Address @craig.topper's comment.

arsenm added a subscriber: arsenm.Jan 9 2023, 2:28 PM
arsenm added inline comments.
llvm/lib/Target/X86/X86InstrCompiler.td
687

Why not go one step further and have a generic barrier machine instr?

reames added inline comments.Jan 9 2023, 2:30 PM
llvm/lib/Target/X86/X86InstrCompiler.td
687

We may. One step at a time. :)

This revision is now accepted and ready to land.Jan 9 2023, 2:56 PM
This revision was landed with ongoing or failed builds.Jan 9 2023, 3:20 PM
This revision was automatically updated to reflect the committed changes.
skan added a subscriber: skan.Jan 21 2023, 12:07 AM