[PATCH] D108903: [llvm-reduce] Add reduce operands pass

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

I'm not sure if we should be using undef, poison, or the null value for reductions. I'll ask llvm-dev for opinions.



================
Comment at: llvm/test/tools/llvm-reduce/remove-operands.ll:1
+; Test that llvm-reduce can reduce operands to their respective null values.
+;
----------------
we should add tests for things like making sure we don't crash on a `br` since basic blocks don't have a null value


================
Comment at: llvm/tools/llvm-reduce/DeltaManager.cpp:53
   DELTA_PASS("instructions", reduceInstructionsDeltaPass)                      \
+  DELTA_PASS("operands", reduceOperandsPass)                                   \
   DELTA_PASS("operand-bundles", reduceOperandBundesDeltaPass)                  \
----------------
`reduceOperandsDeltaPass` for consistency


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceOperands.cpp:31
+  switch (Ty->getTypeID()) {
+  case Type::IntegerTyID:
+  case Type::HalfTyID:
----------------
can we list types that don't have a null value instead? seems better than the current approach since the list is much smaller

I'll run this patch over a larger file to see if we're missing some types in this check (as in we're trying to get the null value of some type that doesn't have a null value)


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceOperands.cpp:62
+        auto *Ty = Op->getType();
+        if (!operandIsNullValue(Op) && typeIsNullable(Ty) && !O.shouldKeep()) {
+          Op.set(Constant::getNullValue(Ty));
----------------
these are always used together, can we create something like `canReduceOperand()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108903