[PATCH] D47816: [InstCombine] Don't sink instructions across inlined function call.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D47816#1123630, @renlin wrote:

> I agree that, generally it is not a good idea to sink across function call. I am a little bit consecutive on this change.
>
> Thank you for your suggestion. I checked the document about TTI::isLoweredToCall. 
>  IIUIC, it seems to me this backend information is still not strong enough.
>
> - If the backend returns false for a function, it will get expanded by some passes somewhere in the optimization pipeline. This might complicated the CFG, similar as the inlined function cases.


In theory, yes. In practice, it shouldn't (unless the backend generates a custom expansion including branches, but in such cases, the costs are generally low). The purpose of the call, however, is to inform the mid-end optimizer whether or not it should view the call as actually having call-like overheads. This applies to information-only intrinsics, calls that get lowered to single instructions (e.g., sin/cos on some targets).

> - If the backend returns true for a function, it is an real function call. This will extend the live-range of related variables.
> 
>   Should we just stop sinking instructions across any type of function call?

No. You would at least need to exclude a whole list of intrinsics. isLoweredToCall will do this for you, and also handle a number of useful cases where the backend expands the call into something cheap. Please use isLoweredToCall, and if we find cases where this turns out to be problematic, we'll figure out how to address them.

> Normally, InstCombine will be called multiple time, if a function got expanded, we still have opportunities to sink instructions when it is beneficial.




Repository:
  rL LLVM

https://reviews.llvm.org/D47816