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().

Ping! 🔔

LGTM, after the minor issues below are addressed.

TargetLoweringBase::isFPImmLegal() could have a default value of false for ForCodeSize.

Commited extra float C99 functions in rGf4a369596f7b.

After the nsz check is addressed, it LGTM.

In D57434#1377569, @Whitney wrote:

@evandro Why do we want to add new cases in test/Transforms/InstCombine/pow-sqrt.ll? Am I right that InstCombine tests opt? This new changes happen in llc.

Separated functions between VC19 and float.

Assume VC19 as the default. However, I wonder if this should be the default earlier, in the clang driver.

Restrict the improvements to VS2015.

In D57625#1386061, @rnk wrote:

Do we know when most of these were added? Did the all arrive in more or less the same version? It would be nice to make this conditional on a new enough environment version. That gives users a way to indicate that they are targetting an old CRT if they need to.

Rebase.

Include ARM.

In D57625#1384950, @mstorsjo wrote:

But the other point still stands; even if compiling LLVM requires VS 2015 or newer, the generated code can work with older MSVC runtimes as well. @rnk - is there any declared minimum for this? I'm using Clang with MSVC 2013 SDK/runtimes just fine in one setup at least (although this setup currently uses Clang/LLVM 4.0, I haven't recently retested it with latest). AFAIK, the exact version of MSVC that is targeted is kept available via Triplet::getEnvironmentVersion, so unless it's intended to drop support for all the older runtimes as well, this would have to check the version of it.

In D57625#1382005, @mstorsjo wrote:

(And it also works with mingw which normally uses an ancient msvcrt.)