core/int128.d: udivmod: don't inline (fixes #4916)

[the-horo]

This works around an issue that makes the code produce seemingly bad results when all of the following condition are met:

1. The code is compiled with -O1 (at least)
2. The code is compiled with -mattr=bmi2
3. The running processor is AMD (reproduced with AMD Ryzen 7 5825U and AMD Ryzen 3960X)

The std.int128 unittests can be used to test if the code was misscompiled.

[kinke]

I can't say I really like this; inlining a single instruction makes perfect sense, if it wasn't for this weird problem on some CPUs.

[the-horo]

Yeah, it's relatively inefficient but I'm also looking for any kind of fix because I don't want to keep this bug in Gentoo, where people do compile their stuff with -march=native...

Interestingly that the testsuite fails on aarch64, I did run it on my RPI and it didn't fail, which, from the way the test is written, makes me question if the testsuite actually ran.

[kinke]

It just fails because I added an explicit codegen check at the asm level for this, making sure that everything is inlined: https://github.com/ldc-developers/ldc/blob/master/tests/codegen/int128.d (always targeting x86_64, on aarch64 too). So that would need an adjustment if we go through with this pessimization.

[the-horo]

Hmm, if the function is no longer inline do you know how we could check its generated assembly? I assume if we don't go through linking then we don't see the code of the non-inline function, but if we go, we no longer have a way to check the instruction.

[kinke]

Yeah exactly, so the 2nd mul_divmod() test function should probably just be removed to get this to pass, it provides no other value, the multiplication is already checked in the first function.

FWIW, I'm still pretty sure this is an LLVM bug, I can't imagine that's really a CPU bug - I did search briefly for divq and CPU bugs, but couldn't find any. Maybe there is a better workaround, such as inserting some dummy instruction afterwards - if the problem is that somehow a divq cannot follow or be followed by a BMI2 instruction, or something along those lines.

[the-horo]

If I had to bet on something, it would definitely be that LLVM is the root cause, but I lack the knowledge to debug it. Perhaps if we were able to get similar C code maybe we would be able to identify what is broken.

[JohanEngelen]

Perhaps only apply the pessimization when __traits(targetHasFeature, "bmi2") == true ?

[kinke]

Ah right, that sounds much better! Let me try it.

Edit: Yep, works totally fine for me, incl. -O -mcpu=native on my Ryzen 3960:

static if (__traits(targetHasFeature, "bmi2"))
    pragma(inline, false);