[PATCH] Caller/Calllee unsafe-fp-math attribute fixup prior to inlining.

[Chandler Carruth]

Chatted a bit about this with Puyan and Eric on IRC. Here is my rough
outline of a plan that I think might *actually* address the various use
cases, and also be a bit cleaner to support long-term in the IR optimizer
(which it sounds like we'll have to).

1) make the unsafe math stuff proper IR attributes so we can reason about
them as such, and define the semantic model for them in the lang ref
1a) auto-upgrade the textual attributes historically used for this to the
enum ones so that we always have these
2) add a 'computeCompatibleAttributeSet(...)' function to
Transforms/Utils/FunctionUtils.{h,cpp} that returns an optional
AttributeSet and accepts (at least) two input AttributeSets (it could also
as a convenience accept Functions and extract their AttributeSets).
2a) have this return *no* attribute set if the textual attributes are not
the same on all of the input sets
2b) have this find a conservatively correct set of the IR attributes for
the unsafe math ones, exactly like Puyan's code already does
3) add some more noisy guidance that textual attributes should only be used
for *target* attributes, not ones for which IR-optimizers are semantically
aware

Then, at some point(s) in the future:
4) Teach the 'computeCompatibleAttributeSet' routine to take an optional
TTI and query it to resolve different textual attributes in the target
(where needed)
5) Refactor some of the existing attribute mismatch detection from various
bizarre parts of the inliner into the 'computeCompatibleAttributeSet'
routine

Thoughts? If this is a viable solution, but too much work right now, can I
help with parts of it?



On Mon, May 5, 2014 at 1:36 PM, Chandler Carruth <chandlerc at google.com>wrote:

> (not sure if it helps, but I'm even willing to help write the code to get
> to one of these better states)
>
>
> On Mon, May 5, 2014 at 12:39 PM, Chandler Carruth <chandlerc at google.com>wrote:
>
>>
>> On Mon, May 5, 2014 at 12:14 PM, Owen Anderson <resistor at mac.com> wrote:
>>
>>> I vehemently disagree with this conclusion.  I agree that, in principle,
>>> we should find a better global solution to the modeling of fast-math flags
>>> (mostly likely by propagating the fine-grained flags through SDISel), but
>>> what you’re proposing is not an acceptable solution.
>>>
>>> 1) Existing functionality is semantically broken.  We will inline
>>> numerically-sensitive procedures into fast-math-enabled functions, likely
>>> breaking their functionality.
>>>
>>
>> No argument from me here. That said, we need to figure out *some* path
>> forward. I would be happy with a lot more in the way of an incremental step
>> if there were some follow-up step coming, but so far I don't see any.
>>
>>
>>>
>>> 2) Refusing to inline mismatched attributes without any attempt to
>>> reconcile them will fundamentally break always_inline in ways that regress
>>> from earlier releases.  This will be at the expense of our users.
>>>
>>
>> Is this true for any textual attributes other than fast-math ones? I
>> can't think of any, and I'm not suggesting this for fast-math flags. I
>> understand the problem there.
>>
>>
>>>
>>> 3) Killing off the global attributes before we’ve threaded this through
>>> SDISel would be an unacceptable performance regression.  A lot of fast math
>>> functionality occurs in the backends, and we *will* dramatically impact
>>> performance of fast math use cases if we do this.
>>>
>>> We should not be rejecting improvements because of a far-distant,
>>> “perfect” vision for which we have no roadmap, and we *definitely*
>>> should not be regressing major use cases!
>>>
>>
>> If there is no roadmap and essentially no one working on actually
>> modeling the instruct flags fully, then fine. We should promote all of the
>> unsafe fp math flags to be formal attributes rather than string attributes,
>> and then model their semantics. They'll be around forever if no one works
>> on it, so we'll have to live without a better solution. By making them full
>> blown attributes we can more reasonably teach the entire optimizer how to
>> interact correctly with them.
>>
>> What I don't want to get the inliner (or any other part of the
>> IR-optimizer) into the business of reasoning in a precise (and thus easily
>> broken) way about the semantics of arbitrary textual attributes. Textual
>> attributes make much more sense when the IR transforms don't need to reason
>> about them in any amount of detail. And I don't want to add "just one" such
>> case for the unsafe fp math flags because it will both invite more (no
>> matter what we say) and be a long term maintenance headache. The textual
>> attributes are intentionally opaque to the IR, so it gets *really* ugly to
>> try and map some semantic onto them.
>>
>>
>> Naturally, I don't want to regress users. =/ I'm just trying to not
>> create still more problems down the road. I don't see any way to do that
>> with textual attributes.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140505/2b234615/attachment.html>