[PATCH] D111818: [llvm-reduce] Introduce operands-skip pass.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceOperandsSkip.cpp:48
+static int classifyReductivePower(Value *V) {
+  if (isa<UndefValue>(V))
+    return 7;
----------------
aeubanks wrote:
> I thought we preferred ConstantData over undef
You mentioned this as "reduce to undef/1/0, probably in that order" in https://reviews.llvm.org/D110274#3062642, but I didn't interpret this as consensus. My approach here was to mimic the current behaviour where everything is reduced to undef if possible. If introducing undef is not desired, then an option could avoid adding it to the set of possibilities. In this approach, it could only be added if there there is already an undef somewhere.

Changing the order of preference might still be better. I changed it accordingly.


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceOperandsSkip.cpp:132
+      // Collect refenced values as potential replacement candidates.
+      SetVector<Value *> &&ReferencedVals = collectReferencedValues(OpVal);
+
----------------
aeubanks wrote:
> is `&&` necessary? the result of `collectReferencedValues()` should be moved into a local variable
It's not necessary, but would save the invocation of move-ctor.


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceOperandsSkip.cpp:142
+      // anymore.
+      std::vector<Value *> &&Candidates = ReferencedVals.takeVector();
+
----------------
removed && here too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111818