[PATCH] D24816: [Target] move reciprocal estimate settings from TargetOptions to TargetLowering

[Mehdi AMINI via llvm-commits]

mehdi_amini added a comment.

> There are independent goals, but I see this is an intermediate step:

> 

> 1. It allows the settings to be different per function.


The technical solution to handle this "per instruction" would be totally different though. These attributes would become obsolete as soon as you have the per instruction solution.
Now if there is not plan to get something per-instruction  in the near future, why not.

What happens during inlining with this patch?

> 2. The settings survive in an LTO build. AFAIK, currently we drop all reciprocal settings with LTO.


This is incidental and shouldn't be a motivation, i.e. making something a property of the function when it is not intrinsically just to "fix" LTO does not seem right to me. The LTO implementation is responsible to properly initialize the backend/codegen,

> 3. It allows https://reviews.llvm.org/rL268539 to be reimplemented. Not sure if there are other barriers for that though?


The post-commit thread is an interesting reading :)

Note: I'm not against this patch, I'm just raising questions, which you may have already considered :)


https://reviews.llvm.org/D24816