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

[Nicholas Guy via Phabricator via llvm-commits]

NickGuy added a comment.

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

It was done to reduce the footprint of this patch to be the interface change only, with any implementation changes in subsequent patches. I'll remove the default and hoist the call site changes up to this patch.

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

I don't think that that's //all// we care about, unfortunately. A number of the backends this change effects check whether a predicate is defined or clobbered, while a few don't seem to have a concept of predicate clobbering (e.g. in R600InstrInfo).

> 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.)

Currently, the Thumb reduction pass is only invoked before IfConversion if we're optimising for size, so we'd prefer to keep the smaller size if possible (this change is an attempt to find a happy middle-ground, getting the performance gain from IfConversion while not sacrificing size)

> I also wonder renaming IncludesRemovable to something like Ignore/SkipDead?

Done, will be included in the next patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88494