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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a subscriber: echristo.
MaskRay added a comment.

In D90554#2408163 <https://reviews.llvm.org/D90554#2408163>, @spatel wrote:

> In D90554#2407132 <https://reviews.llvm.org/D90554#2407132>, @MaskRay wrote:
>
>> define dso_local void @baz() {
>> bb:
>>
>>   %tmp = call <2 x double> @llvm.experimental.constrained.fadd.v2f64(<2 x double> undef, <2 x double> undef, metadata !"round.dynamic", metadata !"fpexcept.ignore")
>>   ret void
>>
>> }
>
> 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?

Hi Sanjay, apologies for the revert churn you have experienced. I have not been with the community long enough, so I'll defer the policy question to @echristo ..

I'll just confirm two facts:

- `llvm.experimental.constrained.fadd.*` is due to -frounding-math. The intrinsics have been in-tree for nearly one year now. I am not clear about its stability.
- At `2583d8eb08073d3c1e06b21b1c4216d0ab7a0909` (the commit before `Temporarily Revert "[CostModel] remove cost-kind predicate for intrinsics in basic TTI implementation"`), cherry picking either the revert or `[CostModel] avoid crashing while finding scalarization overhead` fixes the crash for the original IR reproduce (very large), so I agree that this issue is independent. The code path causing the assertion failure was not exercised before this patch, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90554