Page MenuHomePhabricator

Provide operand indices to adjustSchedDependency
ClosedPublic

Authored by frasercrmck on Mar 31 2020, 4:12 AM.

Details

Summary

This allows targets to know exactly which operands are contributing to
the dependency, which is required for targets with per-operand
scheduling models.

Required, at least, for our downstream target. I added some seemingly-relevant people from AMDGPU and Hexagon to see if they agree with the idea.

Also, I was tempted to change AMDGPU's and Hexagon's use of Src & Dst to Def & Use, as are used in the base method. The discrepancy could be a little confusing.

Diff Detail

Event Timeline

frasercrmck created this revision.Mar 31 2020, 4:12 AM
foad added a comment.Mar 31 2020, 6:21 AM

Seems reasonable to me.

Also, I was tempted to change AMDGPU's and Hexagon's use of Src & Dst to Def & Use, as are used in the base method. The discrepancy could be a little confusing.

That sounds fine too.

foad added inline comments.Mar 31 2020, 6:41 AM
llvm/include/llvm/CodeGen/TargetSubtargetInfo.h
229–234

Why int? unsigned would be more usual.

frasercrmck added inline comments.Mar 31 2020, 7:28 AM
llvm/include/llvm/CodeGen/TargetSubtargetInfo.h
229–234

Good question. I went with int because there's already the idom that -1 means operand not found. I suspect there could be cases where an operand isn't known, possibly for a non-data dependency. In that case we could still allow the passing of -1.

If we go with this idea, I'd suggest we update the documentation to make that clearer.

Updating Hexagon to use Def and Use terminology now seems too an intrusive change to me: there are also private functions updateLatency, restoreLatency`, changeLatency, isBestZeroLatency which all use Src and Dst. So I think I'll leave it be.

AMDGPU is simpler in that regard.

Add documentation for adjustSchedDependency

Rename variables in AMDGPU's adjustSchedDependency, to better match the naming convention used by the base method.

frasercrmck marked an inline comment as done.Apr 3 2020, 2:48 AM
frasercrmck added inline comments.
llvm/include/llvm/CodeGen/TargetSubtargetInfo.h
229–234

I've updated the documentation to describe how DefOpIdx and UseOpIdx may be used.

foad accepted this revision.Apr 16 2020, 2:29 AM

LGTM.

This revision is now accepted and ready to land.Apr 16 2020, 2:29 AM
This revision was automatically updated to reflect the committed changes.