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

Detect local label assignments as directives on MIPS (#6392) #6467

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jf2048
Copy link

@jf2048 jf2048 commented May 11, 2024

For obscure reasons (see comment here) GCC MIPS inserts DWARF debug labels with the format $LXX = . instead of $LXX:. So this updates the regexes in that scenario using the existing MIPS detection, to explicitly consider that specific pattern as a directive instead of an assignment. That way they are filtered correctly in the assembly output with the "Directives" option like on all other architectures.

This shouldn't need any additional tests, just updating the existing ones to say those labels should now be filtered. Currently it causes test failures though, I couldn't find any testing docs or if there is an automatic way to regenerate the test files? If there is a recommended way, I can update them.

@jf2048 jf2048 marked this pull request as draft May 11, 2024 05:00
@partouf
Copy link
Contributor

partouf commented May 13, 2024

I would like to some caution in terms of the tests failing:

The rest of the failing tests it seems Ok to hide the labels.

We've had a bit of shuffling around as the tests are concerned, but you can run npx vitest test/filter-tests.ts --update (according to #6366)

@jf2048 jf2048 force-pushed the mips-labels branch 2 times, most recently from 4cddd03 to 757571b Compare May 15, 2024 15:57
@jf2048 jf2048 marked this pull request as ready for review May 15, 2024 15:58
@jf2048
Copy link
Author

jf2048 commented May 15, 2024

Good point 😃 I managed to fix those tests and find a much simpler solution. Instead of messing about with directives I made it so symbol = . is now detected as a label in all cases. Note that will also cause those lines to be filtered if someone inserts that as inline assembly, but that should be consistent with how unused symbol: lines are handled too if those are placed in inline assembly.

Edit: I guess if someone really wants to put these in manually and always show them, things like symbol = . + 0 could still be used as a workaround.

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

Successfully merging this pull request may close these issues.

None yet

2 participants