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

Valgrind reports unhandled instruction #1677

Closed
langefa opened this issue Dec 15, 2023 · 15 comments
Closed

Valgrind reports unhandled instruction #1677

langefa opened this issue Dec 15, 2023 · 15 comments

Comments

@langefa
Copy link

langefa commented Dec 15, 2023

When running Valgrind on an example for the FUEL library (https://gitlab.com/feynmanintegrals/fuel), it encounters an unhandled instruction:

==903142== Memcheck, a memory error detector
==903142== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==903142== Using Valgrind-3.21.0 and LibVEX; rerun with -h for copyright info
==903142== Command: ./test --calc flint --file example_coeffs/huge_but_short.data
==903142== 
vex amd64->IR: unhandled instruction bytes: 0x62 0x73 0xB5 0x48 0x3A 0xD4 0x1 0xC4 0x43 0xD
vex amd64->IR:   REX=0 REX.W=0 REX.R=0 REX.X=0 REX.B=0
vex amd64->IR:   VEX=0 VEX.L=0 VEX.nVVVV=0x0 ESC=NONE
vex amd64->IR:   PFX.66=0 PFX.F2=0 PFX.F3=0
==903142== valgrind: Unrecognised instruction at address 0x513fd46.
==903142==    at 0x513FD46: mpoly_void_ring_init_fmpz_mpoly_ctx (void_ring.c:113)
==903142==    by 0x5123EEA: fmpz_mpoly_set_str_pretty (set_str_pretty.c:24)
==903142==    by 0x14B359: rational_function (ratfunc_flint.cpp:1212)
...

This did not happen with v3.0.1. However, to fix #1652, FUEL was updated to commit 3ef8253. I checked that the problem persists with the most recent commit 2318e1b in trunk.

I noticed this in the context of looking into a memory leak in FUEL, see https://gitlab.com/feynmanintegrals/fuel/-/issues/1 and #1676.

@albinahlback
Copy link
Collaborator

I'm bad at valgrind -- do I understand it correctly that the unrecognized instruction is in fmpz_mpoly/void_ring.c at line 113 in FLINT?

@albinahlback
Copy link
Collaborator

What CFLAGS were used when you compiled it with 3.0.1, and what CFLAGS are being used now? Are they the same or not?

@Sander80
Copy link

I'm bad at valgrind -- do I understand it correctly that the unrecognized instruction is in fmpz_mpoly/void_ring.c at line 113 in FLINT?

Yes, it pritnt a backtrace of calls, the top line is the inner call in the stack

@Sander80
Copy link

Unless @langefa changed something, it is should be by default, and the flags should be the same. We only pass paths to gmp and mpfr

@albinahlback
Copy link
Collaborator

Okay, so the default CFLAGS where changes some time after that. We increased the optimization level from -O2 to -O3 and added -march=native (unless cross-compiling). It may be the case that Valgrind simply does not recognize the instruction that were generated (see https://stackoverflow.com/questions/37032339/valgrind-unrecognised-instruction for example).

@Sander80
Copy link

Is there any recomended way to force specific CFLAGS? I will force their usage in FUEL, so that @langefa can retry (I have not seen the error message)

@albinahlback
Copy link
Collaborator

Assuming you are using Make to build FLINT, yes. For instance,

$ ./configure CFLAGS="-O2"

will set CFLAGS to -O2, and there may be some other default options that may be put in there too (for instance, --enable-debug is the default, so -g will also be added to the CFLAGS unless you specify --disable-debug).

@Sander80
Copy link

@albinahlback , thank ypu
@langefa , I pushed a commit so that fuel will configure flint with -O2, please give it a try

@langefa
Copy link
Author

langefa commented Dec 15, 2023

The CFLAGS are indeed the culprit. -O3 has no impact though, -march=native is solely responsible. It looks like there is some architecture dependent optimization on my machine that is not compatible with Valgrind.

@Sander80
Copy link

What is the resolution then? Nothing wrong here in flint, the issue can be closed?
I am probably then returning default opitons and you can change them if needed

@fredrik-johansson
Copy link
Collaborator

This is a common problem with valgrind. Before closing the issue, we should add a section to the FLINT docs on using valgrind including a mention of this problem.

@Sander80
Copy link

I see, then it's good we reported it.

@albinahlback
Copy link
Collaborator

The system you built this for, does it support AVX512? I seem to disassemble the instruction to vinserti64x4, which is an AVX512 instruction which seems to be unsupported by Valgrind to-this-day.

@langefa
Copy link
Author

langefa commented Oct 14, 2024

Yes, it does. Thanks for digging into this!

@albinahlback
Copy link
Collaborator

Superseded by #2092.

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

No branches or pull requests

4 participants