A set of function attributes is required in any function that uses constrained floating point intrinsics. None of our tests use these attributes. I've got IR verifier changes in the works that enforce use of them, but first the existing tests need to be made compliant.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This looks fine to me, assuming we agree that this is in fact the set of attributes we need. So I think https://reviews.llvm.org/D67839 should go in first.
Would it be better to do an attributes #0 = { ...} style change? E.g.:
define double @f5(double %a) #0 { ... } attributes #0 = {strictfp noimplicitfloat}
I like that syntax since it is much more compact. I imagine I'll need to update the tests after D67839 is concluded. I'll give this syntax a spin then.
Based on the conclusion from D67839, I've updated the tests to have function attribute strictfp always be used when constrained intrinsics are used, and all function calls are also marked strictfp when the function itself is marked strictfp.
There are still three tests that aren't correct. Those will be subsequent patches.
The verifier changes are in D68233. It can't go in until all tests are passing with it.
I assume you've tested this with your verifier changes. This is a bit too much to verify every call and function manually. I looked at the ones that weren't boilerplate updates, and I sampled the boilerplate changes. Based on that review strategy, it looks good to me.