[PATCH] D77523: Add CanonicalizeFreezeInLoops pass

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune marked 2 inline comments as done.
aqjune added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CanonicalizeFreezeInLoops.cpp:71
+  // Maintains (freeze inst, ind var, stepping instruction, step value)
+  SmallVector<std::tuple<FreezeInst *, PHINode *, BinaryOperator *, Value *>, 4>
+      Candidates;
----------------
jdoerfert wrote:
> aqjune wrote:
> > jdoerfert wrote:
> > > Is there a good reason not to make this a struct with named members so the values and use sites have meaning?
> > Tuple was used because its types seemed to represent what they stand for.
> Sure, a freeze inst, a phi node, a binary operator and a value. You have to see `Value *StepValue = std::get<3>(Candidate);` to guess what the value is. If you want to access the phi (as an Instruction or auto) you have to go back to the vector definition to determine the index. These are (IMHO) good reason against a tuple. Are there any benefits?
Had no special preference for it, so moved to a struct with names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77523