[PATCH] D90554: [CostModel] remove cost-kind predicate for intrinsics in basic TTI implementation

[Sanjay Patel via llvm-commits]

On Fri, Nov 20, 2020 at 2:07 PM Eric Christopher <echristo at gmail.com> wrote:

>
>
> On Fri, Nov 20, 2020 at 1:44 PM Mehdi AMINI via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> mehdi_amini added a comment.
>>
>> In D90554#2408163 <https://reviews.llvm.org/D90554#2408163>, @spatel
>> wrote:
>>
>> > So to clarify policy, we reverted patches when (1) a test based on an
>> experimental intrinsic was crashing and (2) that crash is easily
>> reproducible independent of this patch as shown here:
>> > 7ae346434 <
>> https://reviews.llvm.org/rG7ae346434a5f51b81ebaeeb50bd5d97666ee288b>
>> >
>> > I understand that we revert first and ask questions later, but should
>> that be the rule for experimental code?
>>
>> In that case it seems like there was user-visible impact for some clang
>> user according to @brooksmoses comment above? It isn't like an arbitrary
>> Fuzzer was plugged to the system. IMO a revert is "low cost" and easy to
>> re-land with a fix here, little downside to do this?
>>
>
> Appreciated for sure. I'm not sure how "experimental" we want to consider
> the rounding math intrinsics at this point. It's an interesting question
> and probably should be raised on llvm-dev. If it were something not user
> visible or really experimental I probably wouldn't have reverted, but given
> it's a fairly common use case for developers to use the option it seemed on
> the "not really as experimental as it sounds" area. I think the intrinsics
> themselves might be experimental in the "we could change these radically"
> rather than "we don't really expect these to work".
>

Yeah, I agree. The fixes in this case were easy because I could just about
tell from the assert and backtrace where the problem was, but it might be
more controversial for some theoretical future problem.
So maybe the real question is when to remove the "experimental" label. For
example, we went years with the reduction intrinsics and recently promoted
them because it was clear they had become part of the standard world. If
the constrained intrinsics are already in mainstream use via clang, then we
should treat them as 1st class IR?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201120/bb766477/attachment.html>