[PATCH] D156445: [BPF] Avoid repeating MI->getOperand(NumDefs) x3

Tamir Duberstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 2 08:52:42 PDT 2023


tamird added a comment.

In D156445#4554232 <https://reviews.llvm.org/D156445#4554232>, @eddyz87 wrote:

> In my opinion, whether or not to repeat `MI->getOperand(NumDefs)` is a matter of personal preference in this case. Current code is ok and this change does not change any related functionality. I suggest to withheld from this change to avoid cluttering commit history.

I seems obvious that readability of code-at-HEAD should be of a higher priority than the readability of the commit history; the extant code will be read many more times than the the commit history for the same piece of code.

The description of this change doesn't claim that there is a change in behavior here; is there documentation that requires all changes to have functional changes?

> Also, I don't understand why you landed this change w/o due process.

As I understand it, the process is not prescriptive about reviews being required. I see many changes (particularly in this area) that have gone in without review. In this case, I tried to follow "due process", but received no engagement whatsoever. Is filibustering an accepted practice?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156445/new/

https://reviews.llvm.org/D156445



More information about the llvm-commits mailing list