Except for a handful of gnats, this is really close to LGTM.

In D69987#1815423, @kito-cheng wrote:

The major issue is should we accept/merge an extension which is not ratified/standardized.

It seems to me that all remarks have already been addressed. Is there anything holding this patch? For it pretty much LGTM.

Overall, it looks nearly good enough. I don't know how the scheduler will behave is interesting cases when there is no ShedRead for each argument in the ins list of the instructions. However, it shouldn't be too difficult to add them in the instruction classes.

Can you please clarify how the existing implementation hurts the scheduler and when fused pairs are then broken?

At first glance, this patch seems sensible, but I'm not sure that D70066 is necessary.

🔔 ¡Ping! 🔔

For the record, the issue reported in PR43617 was fixed by rL374243 and a test case was added by rL374453.

Ping! 🔔

Added the hex FP constants for easier auditing.

@efriedma, you had a point. I now trimmed the precision down to one digit short of seeing a change in the mantissa bits.

Indeed, it seemed to be a coarse check in case the following transformations end up calling pow(). However, the only transformation that calls pow() is when shrinking to powf(), which itself chacks for the availability of this routine. So this patch seems to address this issue the wrong way. It seems that removing this check would be better.

In D68257#1690565, @xbolva00 wrote:

Maybe take std::numbers implementation - llvm::numbers?

Split the changes to the AMDGPU target in a separate patch.

Thank you.

Ping! 🔔

🤦🏻‍♂️

Update the patch Including the suggested refactoring.

Thank you.

Running llc -O1 3 times on the byte code from a.i in the "reduced some more" archive:

• Before: average of 38.383s ± 0.064s
• After: average of 38.440s ± 0.049s

Please, stand by for numbers.

Is there a test case that checks that this change does not break what the code in AArch64DAGToDAGISel::Select() was meant to handle?