[PATCH] D155307: [InstCombine] Allow KnownBits to be propagated

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:926
+              !match(I->getOperand(0), m_SpecificInt(LHSKnown.One))) {
+            Constant *NewLHS = Constant::getIntegerValue(VTy, LHSKnown.One);
+            replaceOperand(*I, 0, NewLHS);
----------------
Don't need the separate NewLHS variable now that you use m_SpecificInt.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:929
             return I;
+          } else {
+            RHSKnown = computeKnownBits(I->getOperand(1), Depth + 1, I);
----------------
nit: No else after return.


================
Comment at: llvm/test/Transforms/InstCombine/2023-07-13-arm-infiniteloop.ll:62
+  ret void
+}
+
----------------
pmatos wrote:
> nikic wrote:
> > pmatos wrote:
> > > nikic wrote:
> > > > Please minimize the test -- I don't see how this second function could possibly be related for example.
> > > I did run llvm-reduce... I wonder if llvm-reduce is not removing  functions. I will take a look.
> > llvm-reduce should be removing functions...
> hummm, you're right it does. Which is why in this case the test cannot be reduced further and there's no infinite loop if I manually remove the function bpf_prog_calc_tag. However, I cannot see straightforwardly why that is.
I don't get an infinite loop with your current test either -- I think your current test might be against something like `-O2` rather than `-passes=instcombine`?

Here's a test that works against just instcombine: https://gist.github.com/nikic/a2c0b936686940ae31ae2b23ec9efbcd (Not reduced)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155307