Fixes https://bugs.llvm.org/show_bug.cgi?id=44234 by adding multiply support to freelyNegateValue(). Only one of the operands needs to be negatible, so this still fits within the framework.
Details
Diff Detail
Event Timeline
llvm/test/Transforms/InstCombine/and-or-icmps.ll | ||
---|---|---|
207 | We have a regression here... the relevant difference is that the %B7 neg gets reassociated to act on %B23 instead, which is then pushed through to the %B11 udiv, which is a sext at that point and gets folded to zext. That somehow prevents other folds. As this is a fuzzer generated test case I'm not particularly worried about this, but can take a closer look if people think this is important. |
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | ||
---|---|---|
940 | Due to the one-use restriction (which prevents degenerating to exponential walks) I'm not really worried about recursion here. It could in theory visit many instructions (if you have a thousand multiplies in a chain...), but will never result in super-linear runtime, which I think is the main concern. But I can add a depth limit if desired. |
Looks like the regression in and-or-icmps.ll wasn't actually caused by the IR difference itself, but the worklist order change caused by performing the negation. After rebasing over D73411 the regression disappeared :)
Not related to this change:
Should we worry about recursion depth here? Probably not a issue in practise..