Thank you.

Ping❗ 🔔🔔🔔

Since exp2(itofp(x) is similarly simplified into ldexp(1.0, x) without accounting for any relaxation, methinks that this simplification should be fine too. Will note this.

In D49273#1619819, @xbolva00 wrote:

Oh right, we miss this for pow of two bases other than 2.0.

Yeah, we need a direct fold.

In D49273#1619772, @xbolva00 wrote:

We have exp to ldexp fold, so we indirectly fold pow integer case to ldexp. I think it is okay as is.

In D49273#1619685, @xbolva00 wrote:

Hello, ldexp seems to be faster here. Did you try it?

pow(2, e) to ldexp(1, e)

Ping! 🔔

🔔 ¡Ping! 🔔

Ping! 🔔

It was actually harder to find a SPEC benchmark that triggered this case. As a matter of fact, it shows up only in CPU2017's 525.x264_r. But I can confirm that it works successfully.

In D64099#1569035, @xbolva00 wrote:

I tested the first patch and the second revision with change to exp2 locally with simple examples to check for correctness and performance - and yes, I didn’t check previous revision with folding so a mistake was made.

I have no access to the SPEC benchmarks.

Since using exp2(log()) was mathematically incorrect, have you run any benchmark that validates the results, such SPEC CPU2006 or CPU2017? If so, what kind of improvement did it register?

Yes, default, whether explicit or implicit, counts too, for the resources used inside the target only care about different addresses. In you example, a typical branch predictor will see up to 3 target addresses. Of course, branch predictors are trained by branches that are actually taken, so only target addresses that are executed count. Which opens the window for future work involving FDO or just the static heuristics.

In D60295#1462585, @hans wrote:

I still think looking at the number of cases isn't that much better than looking at the size of the range though. As you said, the point is to limit the load on the branch target predictor, and IIUC that's limited on the number of *different branch targets*, which is really orthogonal to the number of cases. I realize that we don't have that information as readily available, but do you agree that limiting the jump table to a certain number of different targets would be a better approach?

In D60295#1458162, @dmgreen wrote:

What does this do for codesize?

In D60295#1456226, @hans wrote:

I also don't see exactly how the semantics of max-jump-table-size are changed with this patch? What am I missing? Can you upload it again with more context maybe?

I'm still collecting data on other architectures, but I've observed improvements between 3 and 15% in some SPEC benchmarks, such as 253.perlbmk and 400.perlbench, and proprietary ones on AArch64.

If anyone could give me a tip on how to avoid the review to be collapsed to just the part of whichever repo was hit first by a commit, I'd appreciate it.

That's a good idea to follow up on, following the pattern of hasOptNone().