[RISCV] Add wrong arch attribute objfile test.

[khchen]

Summary

arch string should depend on function attribute
in IR target-feature if missing -mattr option.

[jrtc27]

No it shouldn't. This is precisely the kind of IR you get if you want to make an IFUNC specialised for IMAFDC.

[khchen]

I got the similar IR during LTO because the clang driver would not pass the -mattr option to lto code generator.

example:

$(clang) -target riscv64-unknown-linux-gnu -flto -O3 gmain.cpp.o -c main.cpp
$(clang) -target riscv64-unknown-linux-gnu -flto -O3 gmain.cpp.o -o Builtins

In addition, backend will adjust the target-features according to function attribute [1][2],
it means the generated function would have +a,+c,+d,+f,+m instructions,
Why do you think the arch value in object file should still be rv64i2p0?

[1] https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp#L76
[2] https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp#L172

[jrtc27]

In D102925#2775509, @khchen wrote:

I got the similar IR during LTO because the clang driver would not pass the -mattr option to lto code generator.

example:

$(clang) -target riscv64-unknown-linux-gnu -flto -O3 gmain.cpp.o -c main.cpp
$(clang) -target riscv64-unknown-linux-gnu -flto -O3 gmain.cpp.o -o Builtins

In addition, backend will adjust the target-features according to function attribute [1][2],
it means the generated function would have +a,+c,+d,+f,+m instructions,
Why do you think the arch value in object file should still be rv64i2p0?

[1] https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp#L76
[2] https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp#L172

Because the target features only apply to that function. See https://godbolt.org/z/a1oTbacn9 and note that different functions have different target-features, but *only* rv64i2p0 should be present in the output. There are two choices here:

1. Each module records its -march= string and those get combined for the attributes section
2. Only the -march= passed at link time is used for the attributes section

I think 2 is just asking for trouble and we'll end up with people linking without specifying the right -march string (could even be using external libraries that may have been compiled with different extensions to their code) and the solution has to be 1.

[khchen]

update test to show asm has F instructions.

This patch is going to fix wrong attribute for a standalone function or
all functions have same target feature when -march string is missing.

But I feel that you are talking about another problem,
something like how to handle different target feature in the same compilation unit or in
different compilation units. This is not what I want to fix.

Do you still feel this fixed is redundant?

[jrtc27]

In D102925#2776234, @khchen wrote:

update test to show asm has F instructions.

This patch is going to fix wrong attribute for a standalone function or
all functions have same target feature when -march string is missing.

But I feel that you are talking about another problem,
something like how to handle different target feature in the same compilation unit or in
different compilation units. This is not what I want to fix.

Do you still feel this fixed is redundant?

Your patch breaks valid use cases like the one I linked. Please re-read what I said where I explain what the only viable solution I can see is.

[khchen]

clang or clang+llc your test with my patch the arch attribute is still rv64i2p0, does it break your use cases?

I found the iFunc definition always in the declared entity in generated IR,
and it seems target-feature attribute for RISCVAsmPrinter comes from the first in function sequence,
so RISCVAsmPrinter would take the ifunc's target-feature to initialize STI.
(https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp#L172)

1. Each module records its -march= string and those get combined for the attributes section

Need to handle something like linking two bitcodes with different module march string, or string is empty in older bitcode.
I think maybe we could just compute the combination march in RISCVAsmPrinter..
I could do it in another patch when I have time.

I want to say again, this patch just want to fix a problem based on current approach,
that RISCVAsmPrinter reads the target-feature to do something, but current behavior is emitting the arch attribute before read it.

[khchen]

D106347 will fix this issue.

llvm/test/CodeGen/RISCV/subtarget-features-obj-bug.ll

+; RUN: llc -o - < %s | FileCheck %s
+; RUN: llc --filetype=obj -o - < %s | llvm-readelf -A - \
+; RUN:   | FileCheck %s -check-prefix=ATTRIBUTE
+
+; ATTRIBUTE: TagName: arch
+; ATTRIBUTE: Value: rv64i2p0
+
+target triple = "riscv64-unknown-linux-gnu"
+
+; ASM includes F standard extension instructions but arch attribute information is only RV64I base instructions
+
+define float @foo1(i32 %a) nounwind #0 {
+; CHECK: fcvt.s.w        ft0, a0
+; CHECK: fmv.x.w a0, ft0
+  %conv = sitofp i32 %a to float
+  ret float %conv
+}
+
+attributes #0 = { "target-features"="+64bit,+a,+c,+d,+f,+m"}