Add -fno-verbose-asm to clang

[verhovsky]

This removes useless comments (namely repeated function/label names, <n>-byte Folded Spill and others) from the generated assembly.

Fixes #5964

[partouf]

This is maybe an Ok change, but calling in @OfekShilon and @mattgodbolt

It might be appropriate to only hide these when comments are filtered out, but not sure

[OfekShilon]

Great find! I wasn't aware of this switch. I looked in the llvm source and indeed it controls only comments, but I'm not sure all are useless. One random example (first in the list): https://github.com/llvm/llvm-project/blob/31774b6a8a88b435ce79f9ba048ef8bb00e2817e/llvm/lib/CodeGen/AsmPrinter/ARMException.cpp#L96-L110

  bool VerboseAsm = Asm->OutStreamer->isVerboseAsm();

  int Entry = 0;
  // Emit the Catch TypeInfos.
  if (VerboseAsm && !TypeInfos.empty()) {
    Asm->OutStreamer->AddComment(">> Catch TypeInfos <<");
    Asm->OutStreamer->addBlankLine();
    Entry = TypeInfos.size();
  }

  for (const GlobalValue *GV : reverse(TypeInfos)) {
    if (VerboseAsm)
      Asm->OutStreamer->AddComment("TypeInfo " + Twine(Entry--));
    Asm->emitTTypeReference(GV, TTypeEncoding);
  }

Can certainly be useful to anyone debugging ARM exceptions.

So I'd say this switch would be activated only when the comments-filter is checked, but that might require a server roundtrip where (I think?) today it isn't required. Do you think that's doable?

[mattgodbolt]

Yeah doing it when comments are filtered out? I dunno. We've had folks asking before for -fverbose-asm to be the default before now, #4130, #297, #1004

I think I'm ok to merge this, but

• perhaps a more sane/unified approach to verbosity
• there's an argument we've used before that "we do whatever the compiler does by default"

[OfekShilon]

Suggestion: maybe add a checkbox specific to this switch, defaulting to checked? Like the fno-discard-value-names in llvm ir :

That one also triggers a re-compilation so I guess it's doable.

[mattgodbolt]

That sounds like a winner, Ofek; and if we do it right fixes a number of those linked issues too!

[partouf]

Interestingly #4130, #297, #1004 are about GCC, and there it's off by default and if on - most comments are removed by comment filter

I don't know, I'm conflicted. The extra checkbox might be the only correct way to go.

[OfekShilon]

Interestingly #4130, #297, #1004 are about GCC, and there it's off by default and if on - most comments are removed by comment filter

I don't know, I'm conflicted. The extra checkbox might be the only correct way to go.

Actually I take it back - after looking at the old issues I think it would be best to indeed link -fverbose-asm/-fno-verbose-asm to the existing 'comments' checkbox.
i.e.: if it's checked - add -fno-verbose-asm to the build switches (for supporting compilers) AND apply the regular comment-cleanup-logic. If it is unchecked - add -fverbose-asm to the build switches (for supporting compilers) and don't apply comment-cleanup-logic.

[verhovsky]

@OfekShilon from some quick testing, -fverbose-asm has no effect on clang but it does make a difference for GCC and the reverse is true, -fno-verbose-asm does nothing for GCC but effects clang's output.

There's no point in adding -fverbose-asm to clang when the Comments box is unchecked, it would just make the "All compilation options" box harder to read and make people wonder why that option is there and any time they spend on figuring it out would be wasted. In regards to adding -fverbose-asm to GCC, I don't want to do that because the comments are mostly noise:

/* Type your code here, or load an example. */
int square(int num) {
    return num * num;
}

@ GNU C17 (GCC) version 15.0.0 20240529 (experimental) (arm-unknown-linux-gnueabihf)
@       compiled by GNU C version 14.1.0, GMP version 6.1.2, MPFR version 4.2.1, MPC version 1.3.1, isl version isl-0.19-GMP

@ GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
@ options passed: -mfloat-abi=hard -mfpu=neon -mtls-dialect=gnu -mthumb -march=armv7-a+simd -g
square:
        @ args = 0, pretend = 0, frame = 8
        @ frame_needed = 1, uses_anonymous_args = 0
        @ link register save eliminated.
        push    {r7}    @
        sub     sp, sp, #12       @,,
        add     r7, sp, #0        @,,
        str     r0, [r7, #4]      @ num, num
@ /app/example.c:3:     return num * num;
        ldr     r3, [r7, #4]      @ tmp116, num
        mul     r3, r3, r3        @ _2, tmp116, tmp116
@ /app/example.c:4: }
        mov     r0, r3    @, <retval>
        adds    r7, r7, #12     @,,
        mov     sp, r7    @,
        @ sp needed     @
        ldr     r7, [sp], #4      @,
        bx      lr  @

vs without -fverbose-asm but with Comments unchecked

square:
        @ args = 0, pretend = 0, frame = 8
        @ frame_needed = 1, uses_anonymous_args = 0
        @ link register save eliminated.
        push    {r7}
        sub     sp, sp, #12
        add     r7, sp, #0
        str     r0, [r7, #4]
        ldr     r3, [r7, #4]
        mul     r3, r3, r3
        mov     r0, r3
        adds    r7, r7, #12
        mov     sp, r7
        @ sp needed
        ldr     r7, [sp], #4
        bx      lr

The line numbers @ /app/example.c:4: } are already in compiler's explorer rainbow highlighting and the options passed is also noise.

[OfekShilon]

@verhovsky I assume you're talking about my suggestion options.push(filters.commentOnly ? '-fno-verbose-asm' : '-fverbose-asm');. Indeed I intended for this to be in BaseCompiler.optionsForFilter and not Clang.optionsForFilter - as said later in the comment. Sorry for being unclear.

I think the small extra-verbosity in the 'all compilation options' is negligible, and also think that if gcc authors added these comments - they aren't noise at least to them. For me personally as a user it is completely acceptable to see some noise when I check some verbosity box like 'comments'. But other opinions are welcome of course - @partouf ? @mattgodbolt ?

[partouf]

@verhovsky I assume you're talking about my suggestion options.push(filters.commentOnly ? '-fno-verbose-asm' : '-fverbose-asm');. Indeed I intended for this to be in BaseCompiler.optionsForFilter and not Clang.optionsForFilter - as said later in the comment. Sorry for being unclear.

I think the small extra-verbosity in the 'all compilation options' is negligible, and also think that if gcc authors added these comments - they aren't noise at least to them. For me personally as a user it is completely acceptable to see some noise when I check some verbosity box like 'comments'. But other opinions are welcome of course - @partouf ? @mattgodbolt ?

my 2cents: we either do it consistently so we can fix this + the 3 gcc/icc issues, or we don't do it at all and leave as is

So I agree with your suggestion @OfekShilon that it should be part of the comments filter

[OfekShilon]

@verhovsky I think adding this check to BaseCompiler.optionsForFilter would impact only clang and gcc, but a safer alternative would be -
(1) add to ClangParser, GCCParser and GCCCParser a hasSupport(options, '-fverbose-asm') check in setCompilerSettingsFromOptions
(2) store it in a new compiler member like supportsVerboseAsm
(3) In BaseCompiler.optionsForFilter refine the added test to something like:

if (this.compiler.supportsVerboseAsm) {
   options.push(filters.commentOnly ? '-fno-verbose-asm' : '-fverbose-asm');
}

You can grep the source for compiler.supports to track creation and usage of other similar members.