[PyTorch] Save a single add instruction in the dispatcher

[swolchok]

Stack from ghstack:

• [PyTorch] Save a single add instruction in the dispatcher #52543 [PyTorch] Save a single add instruction in the dispatcher

This saves one (1) add instruction. New code comments should
explain exactly why. In short, we store a direct pointer in
OperatorHandle in addition to the std::list<OperatorDef>::iterator
because converting the latter to the former requires an add instruction.

It is not clear to me whether this is a particularly great tradeoff,
but I spent (more) time on it (than I expected), so here it is for
review.

Differential Revision: D26564192

[facebook-github-bot]

💊 CI failures summary and remediations

As of commit 0e15d1f (more details on the Dr. CI page):

---

• 2/2 failures possibly* introduced in this PR
  • 1/2 non-scanned failure(s)

---

1 failure not recognized by patterns:

Job	Step	Action
flake8-py3	Add annotations	🔁 rerun

---

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

[codecov]

Codecov Report

Merging #52543 (0e15d1f) into gh/swolchok/136/base (7ca9776) will decrease coverage by 0.00%.
The diff coverage is 86.20%.

@@                   Coverage Diff                    @@
##           gh/swolchok/136/base   #52543      +/-   ##
========================================================
- Coverage                 80.66%   80.66%   -0.01%     
========================================================
  Files                      1967     1967              
  Lines                    215898   215900       +2     
========================================================
- Hits                     174154   174153       -1     
- Misses                    41744    41747       +3     

[smessmer]

Feeling ambivalent about this. From a code style point of view, storing the same data twice is bad since it introduces potential inconsistencies. But if it saves an instruction and that's worth it, then go ahead.

[bhosmer]

Feeling ambivalent about this. From a code style point of view, storing the same data twice is bad since it introduces potential inconsistencies. But if it saves an instruction and that's worth it, then go ahead.

@smessmer I think we need to balance the two kinds of redundancy though. E.g. precomputing the dispatch table is similarly redundant, in the sense that they both store intermediate results for later use. And we're doing that for essentially the same reason as this (to save a ton of redundant computation).

Also from a human standpoint, accessing operator properties through the def rather than the iterator is much clearer. I mean arguably, the redundant member we're storing to save computation is the iterator rather than the definition, right?

ofc this is all pending Scott's confirmation of the effect on instructions. But if the 1% perf bump holds up, that's nothing to sneeze at.

[swolchok]

I rechecked the assembly diff. There is some noise because register allocation gets perturbed and different things end up getting spilled to the stack. However, the net savings is indeed one instruction. Landing.

[facebook-github-bot]

This pull request has been merged in f5e7255.