This is an archive of the discontinued LLVM Phabricator instance.

Only export STATEPOINT results if used in nonlocal blocks.
ClosedPublic

Authored by dantrushin on Apr 26 2022, 4:04 AM.

Details

Summary

Cuurently we always export STATEPOINT results (GC pointers lowered via VRegs)
to virtual registers. When processing gc.relocate instructions we have to
generate CopyFromRegs node and then export it to VReg again if gc.relocate
is used in other basic blocks. This results in generation of extra COPY MIR
instruction if statepoint and its gc.relocate are in same BB, but gc.relocate
result is used in other blocks.

This patch changes this behavior to export statepoint results only if used
in other basic blocks. It also records gc.relocate SDValue mapping for local
uses (it is equal to corresponding STATEPOINT result). Doing this gc.relocate
mapping in LowerStatepoint() allows to avoid complicated data structures which
need to be maintained across BB boundaries.

This is NFC and is purely compile time optimization. On big methids it can improve
codegen compile time up to 10%.

Diff Detail

Event Timeline

dantrushin created this revision.Apr 26 2022, 4:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2022, 4:04 AM
dantrushin requested review of this revision.Apr 26 2022, 4:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2022, 4:04 AM

Realized we can s/Value2Res/LowerAsVReg/g

reames added inline comments.Apr 26 2022, 8:10 AM
llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
899

The idiomatic way to manipulate NodeMap is via setValue. Please use that.

See also comment about placement below.

1231

I believe it would be much more idiomatic to recognize that the gc.relocate is local here, and directly setValue the appropriate statepoint result. I'm not really sure what the implications of calling setValue in the "wrong" spot are either.

This would essentially require adding a new state to the record.type field, and handling local cases explicitly.

Introduce new relocation type and use it to lower local gc.relocates.

dantrushin marked 2 inline comments as done.Apr 26 2022, 11:01 AM
reames accepted this revision.Apr 26 2022, 11:32 AM

LGTM

This revision is now accepted and ready to land.Apr 26 2022, 11:32 AM