-mattr should be encoded in the IR for LTO's sake

[twd2]

Bugzilla Link	51161
Version	unspecified
OS	Linux
Blocks	#4440
Attachments	The diagnostic information.
CC	@asb,@efriedma-quic,@fhahn,@MaskRay,@nathanchance,@nickdesaulniers,@smithp35,@twd2

Extended Description

When compiling the following code with LTO:

$ cat test.c
void _start()
{
    asm volatile (
        ".balign 4\n\t"
        "c.nop\n\t"
        ".balign 4\n\t"
        "c.nop"
    );
}

$ clang -march=rv64imac -O2 -flto -mno-relax -c test.c
$ ld.lld test.o

lld fails with unable to write nop sequence of 2 bytes.
The diagnostic information is attached.

It looks like to be due to the HasStdExtC check failure in RISCVAsmBackend::writeNopData.
And, if I use ld.lld -mllvm -mattr=+c test.o, lld can generate proper machine code.
According to Nick Desaulniers [1], -mattr should be encoded in the IR for LTO's sake.

My LLVM version: commit 1b61d83 ("[Inline] Add test for #49933 (NFC)")
[1] https://lore.kernel.org/linux-riscv/CAKwvOdmNji0AbYUiOSfb5cLD+g7YCpXk4oDupa8gTfgzYmxvBg@mail.gmail.com/

[fhahn]

Created attachment 25046 [details]

It looks like to be due to the HasStdExtC check failure in
RISCVAsmBackend::writeNopData.
And, if I use ld.lld -mllvm -mattr=+c test.o, lld can generate proper
machine code.
According to Nick Desaulniers [1], -mattr should be encoded in the IR for
LTO's sake.

Yes it should, but you are not passing -mattr=+c to the clang invocation. Does it work if you add -mattr=+c to your clang line? I assume the +c feature is not part of -march=rv64imac

[twd2]

Created attachment 25046 [details]

It looks like to be due to the HasStdExtC check failure in
RISCVAsmBackend::writeNopData.
And, if I use ld.lld -mllvm -mattr=+c test.o, lld can generate proper
machine code.
According to Nick Desaulniers [1], -mattr should be encoded in the IR for
LTO's sake.

Yes it should, but you are not passing -mattr=+c to the clang
invocation. Does it work if you add -mattr=+c to your clang line? I assume
the +c feature is not part of -march=rv64imac

Hi, I don't know how to pass -mattr=+c to clang:

$ clang -march=rv64imac -mattr=+c -O2 -flto -mno-relax -c test.c
clang-13: error: unknown argument: '-mattr=+c'

But, rv64imac actually implies the +c feature:

llvm-project/clang/lib/Driver/ToolChains/Arch/RISCV.cpp

Line 417 in f1cbea3

case 'c':

[fhahn]

But, rv64imac actually implies the +c feature:
https://github.com/llvm/llvm-project/blob/
f1cbea3/clang/lib/Driver/ToolChains/Arch/
RISCV.cpp#L417

Strange, the target feature gets added as expected when looking at Godbolt: https://godbolt.org/z/4avz69d3v

Can you check the IR in test.o and see if it has "target-features"="...,+c,..." in the attributes of the function?

[twd2]

But, rv64imac actually implies the +c feature:
https://github.com/llvm/llvm-project/blob/
f1cbea3/clang/lib/Driver/ToolChains/Arch/
RISCV.cpp#L417

Strange, the target feature gets added as expected when looking at Godbolt:
https://godbolt.org/z/4avz69d3v

Can you check the IR in test.o and see if it has
"target-features"="...,+c,..." in the attributes of the function?

My output is similar to yours:

$ clang -march=rv64imac -O2 -flto -mno-relax -c test.c
$ test llvm-dis test.o -o -       
; ModuleID = 'test.o'
source_filename = "test.c"
target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n64-S128"
target triple = "riscv64-unknown-linux-gnu"

; Function Attrs: nounwind
define dso_local void @​_start() local_unnamed_addr #​0 {
  tail call void asm sideeffect ".balign 4\0A\09c.nop\0A\09.balign 4\0A\09c.nop", ""() #​1, !srcloc !​6
  ret void
}

attributes #​0 = { nounwind "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+64bit,+a,+c,+m,-relax,-save-restore" }
attributes #​1 = { nounwind }

!llvm.module.flags = !{#0, !​1, !​2, !​3, !​4}
!llvm.ident = !{#5}

!​0 = !{i32 1, !"wchar_size", i32 4}
!​1 = !{i32 1, !"target-abi", !"lp64"}
!​2 = !{i32 1, !"SmallDataLimit", i32 8}
!​3 = !{i32 1, !"ThinLTO", i32 0}
!​4 = !{i32 1, !"EnableSplitLTOUnit", i32 1}
!​5 = !{!"clang version 13.0.0"}
!​6 = !{i32 49, i32 61, i32 81, i32 105}

^0 = module: (path: "test.o", hash: (0, 0, 0, 0, 0))
^1 = gv: (name: "_start", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 1, live: 0, dsoLocal: 1, canAutoHide: 0), insts: 2))) ; guid = 9315384113491396937
^2 = flags: 8
^3 = blockcount: 1
$ test ld.lld test.o 
LLVM ERROR: unable to write nop sequence of 2 bytes

The target-features attributes seem to be ignored by lld (?)

[efriedma-quic]

I think this is what's happening:

1. You pass the "c" feature to clang.
2. clang encodes "c" feature into the target-features of each function.
3. You pass the bitcode into lld.
4. The RISCV asm printer knows it's in a "c" function, and adjusts... but never actually passes the information on to the MC layer.
5. MC layer doesn't realize compressed ops are supposed to be legal, and prints an error.

Probably the answer here is that the RISCV asmprinter should be calling emitDirectiveOptionRVC() somewhere, so the output is unambiguous.

[twd2]

This code doesn't work as well:

void _start(void)
{
    asm volatile (
        ".option push\n\t"
        ".option rvc\n\t"
        ".balign 4\n\t"
        "c.nop\n\t"
        ".balign 4\n\t"
        "c.nop\n\t"
        ".option pop"
    );
}

[twd2]

I find the following patch is related to this issue:
https://reviews.llvm.org/D45962

[twd2]

I find the above PR is merged, and after some tests, I think this issue is solved.

[nickdesaulniers]

Thanks for the follow up; closing.

[nickdesaulniers]

This came up again in ClangBuiltLinux/linux#1942, and is causing issues reported in #65090. Reopening.