[PATCH] D88494: Add "IncludeRemovable" parameter to TargetInstrInfo::DefinesPredicate

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I guess under this your classification, there are basically three possible kinds of predicable instruction:

1. The instruction doesn't define a predicate
2. The instruction defines a predicate, but the predicate is dead, and predicating it will make the definition go away.
3. The instruction defines a predicate, and it will continue to define a predicate after predication.

If the boolean is specified, we check for 3, but not 2?

Do we really need the boolean to default to false?  There are only two callers, as far as I can tell.

Can we rename the function to something like "ClobbersPredicate", if that's what we actually care about here?

I'm a little concerned IfConversion isn't really prepared to deal with the distinction here: in some cases, if conversion doesn't predicate all the instructions in the if-converted block.

For Thumb, would it ever make sense to undo size reduction if it would unblock if-conversion?  (Off the top of my head, I guess this would only have an impact if we wanted to predicate something like an "adds" with a predicate that isn't dead, so maybe not worth investing any effort.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88494