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

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 19 08:11:13 PST 2018


hfinkel accepted this revision.
hfinkel added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/Transforms/Scalar/BDCE.cpp:130
       Changed = true;
     }
   }
----------------
nikic wrote:
> 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.
> 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.

I agree (the old logic looks like it won't do this, and that seems to be a mistake). If you could do this as follow-up, that would be great.

LGTM.


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

https://reviews.llvm.org/D55563





More information about the llvm-commits mailing list