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.

Sign up for GitHub

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

Jump to bottom

[BUG]: armv8-a clang doesn't filter out function comment #5964

Open
verhovsky opened this issue Jan 10, 2024 · 4 comments · May be fixed by #6462
Open

[BUG]: armv8-a clang doesn't filter out function comment #5964

verhovsky opened this issue Jan 10, 2024 · 4 comments · May be fixed by #6462
Labels
bug

Comments

@verhovsky
Copy link
Contributor

Describe the bug

If you compile the example C program

int square(int num) {
    return num * num;
}

with armv8-a clang (any version I tried trunk and the oldest one) with the "Comments" filter enabled, it compiles to

square(int):                             // @square(int)
        sub     sp, sp, #16
        str     w0, [sp, #12]
        ldr     w8, [sp, #12]
        ldr     w9, [sp, #12]
        mul     w0, w8, w9
        add     sp, sp, #16
        ret

that "// @square(int)" shouldn't be there. The comment filter is probably getting tripped up because it's on the same line as the label.

Compare armv7-a clang:

square(int):
        sub     sp, sp, #4
        str     r0, [sp]
        ldr     r0, [sp]
        mul     r1, r0, r0
        mov     r0, r1
        add     sp, sp, #4
        bx      lr

Steps to reproduce

  1. Visit godbolt
  2. Set language to C
  3. Set compiler to armv8-a clang

Expected behavior

square(int):
        sub     sp, sp, #16
        str     w0, [sp, #12]
        ldr     w8, [sp, #12]
        ldr     w9, [sp, #12]
        mul     w0, w8, w9
        add     sp, sp, #16
        ret

Reproduction link

https://compiler-explorer.com/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,selection:(endColumn:2,endLineNumber:4,positionColumn:2,positionLineNumber:4,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:'//+Type+your+code+here,+or+load+an+example.%0Aint+square(int+num)+%7B%0A++++return+num+*+num%3B%0A%7D'),l:'5',n:'0',o:'C%2B%2B+source+%231',t:'0')),k:50,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:compiler,i:(compiler:armv7-clang1101,filters:(b:'0',binary:'1',binaryObject:'1',commentOnly:'0',debugCalls:'1',demangle:'0',directives:'0',execute:'1',intel:'0',libraryCode:'0',trim:'1'),flagsViewOpen:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,libs:!(),options:'',overrides:!(),selection:(endColumn:19,endLineNumber:8,positionColumn:19,positionLineNumber:8,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:1),l:'5',n:'0',o:'+armv7-a+clang+11.0.1+(Editor+%231)',t:'0')),k:50,l:'4',n:'0',o:'',s:0,t:'0')),l:'2',n:'0',o:'',t:'0')),version:4

Screenshots

Not applicable

Operating System

No response

Browser version

No response
@verhovsky verhovsky added the bug label Jan 10, 2024
@verhovsky verhovsky mentioned this issue Jan 10, 2024
Closed
@partouf
Copy link
Contributor

partouf commented Jan 10, 2024

Despite it being called 'comments', the filter actually only removes lines that are "just comments"

So this is working as expected

@verhovsky
Copy link
Contributor Author

verhovsky commented Jan 10, 2024 •
edited

I don't need to be told the name of the label twice on the same line.

I can add -fno-verbose-asm but most users won't know about that option. For ease of use it should be the default (to reduce visual noise) or surfaced in the UI as a toggle maybe?

@OfekShilon
Copy link
Member

Despite it being called 'comments', the filter actually only removes lines that are "just comments"

So this is working as expected

That's technically true, but since armv8-a clang added some visual clutter of the type our filters aim to clean - it makes since to extend them to this case now. No?

@OfekShilon
Copy link
Member

Actually I'm not entirely sure:
Perhaps typical asm EOL-comments hold information that is useful in more contexts than typical full-line comments. Eg here full-line comments are adjacent to debug directives, like

// DW_AT_export_symbols

but EOL comments say something about exhausting registers, like:

        stp     x29, x30, [sp, #48]             // 16-byte Folded Spill

@partouf WDYT? Do these qualify as visual-noise that we wish to declutter by default? (or maybe provide a different option to filter out?)

@verhovsky verhovsky linked a pull request May 9, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add -fno-verbose-asm to clang verhovsky/compiler-explorer
3 participants
@OfekShilon @partouf @verhovsky