Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCC miscompiles Fp2 with --opt:size flag #229

Closed
mratsim opened this issue Apr 17, 2023 · 1 comment · Fixed by #231
Closed

GCC miscompiles Fp2 with --opt:size flag #229

mratsim opened this issue Apr 17, 2023 · 1 comment · Fixed by #231
Labels

Comments

@mratsim
Copy link
Owner

mratsim commented Apr 17, 2023

From #228,

This fails

nim c --cc:gcc -r -d:release --opt:size --outdir:build tests/math/t_fp2.nim

Those do not

nim c --cc:clang -r -d:release --opt:size --outdir:build tests/math/t_fp2.nim
nim c --cc:gcc -r -d:release --outdir:build tests/math/t_fp2.nim

The function that is miscompiled is

func mul_fp2_complex_asm_adx*(
r: var array[2, Fp],
a, b: array[2, Fp]
) =
## Complex multiplication on 𝔽p2
var d {.noInit.}: array[2,doublePrec(Fp)]
d.mul2x_fp2_complex_asm_adx(a, b)
r.c0.mres.limbs.redcMont_asm_adx_inline(
d.c0.limbs2x,
Fp.fieldMod().limbs,
Fp.getNegInvModWord(),
Fp.getSpareBits()
)
r.c1.mres.limbs.redcMont_asm_adx_inline(
d.c1.limbs2x,
Fp.fieldMod().limbs,
Fp.getNegInvModWord(),
Fp.getSpareBits()
)

@mratsim
Copy link
Owner Author

mratsim commented Apr 18, 2023

Following further investigation in #230, I believe that the compiler finds out that M is a constant by constant folding through compilation units with LTO and generating new procedures with only the variable input
As the inline assembly directly use the pointer as an input register, it's likely that its value has been hardcoded. This wouldn't be a problem if the address of the constant didn't change but who knows what the linker does (and there is relocatable code and Address Space Layout Randomization as well)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant