[PATCH] D55563: [BDCE][DemandedBits] Detect dead uses of undead instructions

[Nikita Popov via Phabricator via llvm-commits]

nikic marked 5 inline comments as done.
nikic added inline comments.


================
Comment at: lib/Analysis/DemandedBits.cpp:455
+  if (UserI->getType()->isIntOrIntVectorTy()) {
+    auto Found = AliveBits.find(UserI);
+    if (Found->second.isNullValue())
----------------
hfinkel wrote:
> We know that this will never return AliveBits.end()? A comment or assert should explain this.
I think that due to the way the API is used by BDCE (checking isInstructionDead first) this case can't be hit, but from the perspective of just this API it could happen. I've added a check.


================
Comment at: lib/Transforms/Scalar/BDCE.cpp:130
       Changed = true;
     }
   }
----------------
hfinkel wrote:
> With this new logic, where do we now remove instructions for which all uses have been trivialized?
BDCE currently (also before this change) does not remove instructions without demanded bits, it only replaces their uses with zero and lets somebody else handle the cleanup (some BDCE tests run instcombine afterwards). Only instructions not reached during analysis are removed entirely (the isInstructionDead check).

I think BDCE *should* be dropping them, but it's probably best to do this as a separate change, as it will require changes to otherwise unrelated BDCE tests.


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

https://reviews.llvm.org/D55563