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

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceOperands.cpp:82
+        if (canReduceOperand(Op)) {
+          VariableCount++;
+        }
----------------
`VariableCount` doesn't really make sense, maybe just `Count` is good enough


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceOperands.cpp:31
+  switch (Ty->getTypeID()) {
+  case Type::IntegerTyID:
+  case Type::HalfTyID:
----------------
swamulism wrote:
> aeubanks wrote:
> > 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)
> I would rather keep it a list of types that work rather than types that don't work. Since if a new type is added that is not nullable in the future this pass would crash.
> 
> But it would probably bet better for this logic to live in the same place as `Constant::getNullValue`
yeah, can you add a `Constant::hasNullValue`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108903