This is an archive of the discontinued LLVM Phabricator instance.

[Statepoint] Factor-out utility function to get non-foldable area of STATEPOINT like instructions. NFC
ClosedPublic

Authored by skatkov on Apr 5 2021, 12:01 AM.

Diff Detail

Event Timeline

skatkov created this revision.Apr 5 2021, 12:01 AM
skatkov requested review of this revision.Apr 5 2021, 12:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2021, 12:01 AM
skatkov added inline comments.Apr 5 2021, 2:07 AM
llvm/include/llvm/CodeGen/TargetInstrInfo.h
1073

to be honest, not happy with the name.
Actually I also want to use this function to determine what operands are ok to use both stack and register...
Any suggestions for the name?

reames requested changes to this revision.Apr 5 2021, 8:41 AM

Naming suggestions, but likely to converge to LGTM on next iteration.

llvm/lib/CodeGen/TargetInstrInfo.cpp
501

Minor, move the assert above the variable declarations.

This revision now requires changes to proceed.Apr 5 2021, 8:41 AM
reames added a comment.Apr 5 2021, 8:46 AM

For unclear reasons, phab dropped a long inline comment from my previous review. Rewritten here.

The comment and naming was confusing to me. I'd suggest the following changes.

  1. Restrict the API and naming to patchpoint, stackmap, and statepoint explicitly.
  2. Be clear about the fact that operands are "potentially foldable", not "guaranteed foldable".

A suggested name and comment: getPatchpointUnfoldableRange.

For a patchpoint, stackmap, or statepoint intrinsic, return the range of operands which can't be folded into stack references. These instructions are unique in that stack references for some operands have the same execution cost (e.g. none) as the unfolded register forms. The ranged return is guaranteed to include all operands which can't be folded at zero cost.

skatkov updated this revision to Diff 335392.Apr 5 2021, 9:10 PM

Handled Philip's omments.

Philip, thank you for your phrasing suggestions.

reames accepted this revision.Apr 5 2021, 9:13 PM

LGTM

This revision is now accepted and ready to land.Apr 5 2021, 9:13 PM