[PATCH] D87342: Allow targets to augment computeKnownBits with their analysis using TargetTransformInfo

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added a comment.

@nikic, @spatel, @lebedev.ri, @RKSimon, thank you for your feedbacks!

> I do have a concern about the general direction here: When we allowed targets to hook into InstCombine, one of the hard design constraints was that the target is only allowed to affect combines for target intrinsics, and nothing else. This patch seems to go against that restriction by allowing to replace general analysis behavior and affect otherwise target-independent folds.

To be honest, I don't really like it either, but given it looks like only us care about precise GEPs, I don't see how we can reconcile both constrains.

That said, this patch doesn't affect the combines per se, since this patch doesn't modify instcombine. It only affects the computeKnownBits analysis (i.e., it is more a complicated version of what in spirit @spatel is suggesting with a dedicated GEP analysis for the precise case.) and I was not planning to have instcombine taking advantage of this. But yes, this opens that door.

> It's not clear to me if there's anything other than the GEP example planning to use an override.

I personally don't plan to have any other override here, but that doesn't mean that other needs may arise in the future.

> Can we create a dedicated GEP analysis function that would limit the compile-time impact?

What do you have in mind?

> Or could we use/create some option that would enable the more expensive analysis selectively?

I have to admit I am not a fan of that because it opens the door of having to do the same for pretty much everything that people would consider too expensive for their target. Put differently why GEPs are different?

Cheers,
-Quentin


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87342