This is an archive of the discontinued LLVM Phabricator instance.

Differential D98881

[RISCV] Fix mcount name
ClosedPublic

Authored by nathanchance on Mar 18 2021, 11:31 AM.

Details

Reviewers
simoncook
asb
lenary
craig.topper
jrtc27
MaskRay
Commits
rGef58ae86ba77: [RISCV] Fix mcount name
Summary

GCC's name for this symbol is _mcount, which the Linux kernel expects in
a few different place:

$ echo 'int main(void) { return 0; }' | riscv32-linux-gcc -c -pg -o tmp.o -x c -

$ llvm-objdump -dr tmp.o | grep mcount
                        0000000c:  R_RISCV_CALL _mcount

$ echo 'int main(void) { return 0; }' | riscv64-linux-gcc -c -pg -o tmp.o -x c -

$ llvm-objdump -dr tmp.o | grep mcount
                000000000000000c:  R_RISCV_CALL _mcount

$ echo 'int main(void) { return 0; }' | clang -c -pg -o tmp.o --target=riscv32-linux-gnu -x c -

$ llvm-objdump -dr tmp.o | grep mcount
                        0000000a:  R_RISCV_CALL_PLT     mcount

$ echo 'int main(void) { return 0; }' | clang -c -pg -o tmp.o --target=riscv64-linux-gnu -x c -

$ llvm-objdump -dr tmp.o | grep mcount
                000000000000000a:  R_RISCV_CALL_PLT     mcount

Set MCountName to "_mcount" in RISCVTargetInfo then prevent it from
getting overridden in certain OSTargetInfo constructors.

Diff Detail

Unit TestsFailed

TimeTest
110 msx64 debian > lld.MachO::dependency-info.s

Event Timeline

nathanchance created this revision.Mar 18 2021, 11:31 AM
Herald added subscribers: vkmr, evandro, luismarques and 10 others. · View Herald TranscriptMar 18 2021, 11:31 AM
nathanchance requested review of this revision.Mar 18 2021, 11:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 18 2021, 11:31 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
nathanchance added reviewers: simoncook, asb, lenary, craig.topper, jrtc27.Mar 18 2021, 11:34 AM
nathanchance added a subscriber: nickdesaulniers.
Harbormaster completed remote builds in B94516: Diff 331641.Mar 18 2021, 1:18 PM
jrtc27 added a comment.Mar 18 2021, 2:19 PM

This should really be for all OSes on RISC-V, which I think means copying it N times? :/

(GCC defines MCOUNT_NAME in riscv/riscv.h, not riscv/linux.h)

nathanchance added a comment.EditedMar 18 2021, 2:38 PM

Would it be better to hoist it into RISCVTargetInfo and modify the BSD/Fuchsia switch statements to respect that in their respective TargetInfo constructors?

nathanchance added a comment.Mar 18 2021, 5:41 PM

Such a diff does not look too bad:

diff --git a/clang/lib/Basic/Targets/OSTargets.h b/clang/lib/Basic/Targets/OSTargets.h
index 539466c4f678..c32972c1e04e 100644
--- a/clang/lib/Basic/Targets/OSTargets.h
+++ b/clang/lib/Basic/Targets/OSTargets.h
@@ -256,6 +256,8 @@ public:
     case llvm::Triple::ppcle:
     case llvm::Triple::ppc64:
     case llvm::Triple::ppc64le:
+    case llvm::Triple::riscv32:
+    case llvm::Triple::riscv64:
       this->MCountName = "_mcount";
       break;
     case llvm::Triple::arm:
@@ -488,6 +490,8 @@ public:
     case llvm::Triple::ppc:
     case llvm::Triple::ppc64:
     case llvm::Triple::ppc64le:
+    case llvm::Triple::riscv32:
+    case llvm::Triple::riscv64:
     case llvm::Triple::sparcv9:
       this->MCountName = "_mcount";
       break;
@@ -885,7 +889,14 @@ protected:
 public:
   FuchsiaTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : OSTargetInfo<Target>(Triple, Opts) {
-    this->MCountName = "__mcount";
+    switch (Triple.getArch()) {
+    case llvm::Triple::riscv32:
+    case llvm::Triple::riscv64:
+      break;
+    default:
+      this->MCountName = "__mcount";
+      break;
+    }
     this->TheCXXABI.set(TargetCXXABI::Fuchsia);
   }
 };
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index abae51e75a19..8df6e05ebcd5 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/clang/lib/Basic/Targets/RISCV.h
@@ -59,6 +59,7 @@ public:
     WCharType = SignedInt;
     WIntType = UnsignedInt;
     HasRISCVVTypes = true;
+    MCountName = "_mcount";
   }
 
   bool setCPU(const std::string &Name) override {
diff --git a/clang/test/CodeGen/mcount.c b/clang/test/CodeGen/mcount.c
index 649f0b56949d..2883a6370d34 100644
--- a/clang/test/CodeGen/mcount.c
+++ b/clang/test/CodeGen/mcount.c
@@ -12,6 +12,14 @@
 // RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple mips64el-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32 -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64 -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv32-freebsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-fuschia -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
+// RUN: %clang_cc1 -pg -triple riscv64-openbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
 // RUN: %clang_cc1 -pg -triple powerpc64le-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
nathanchance updated this revision to Diff 332811.Mar 23 2021, 3:41 PM
nathanchance retitled this revision from [RISCV] Fix mcount name for Linux to [RISCV] Fix mcount name.
nathanchance edited the summary of this revision. (Show Details)
  • Hoist MCountName assignment into RISCVTargetInfo
  • Prevent MCountName from getting assigned in FreeBSD and OpenBSD targets
  • Add more tests

I did not update Fuchsia or any other operating systems because it appears that the only supported configurations from GCC are -elf, -linux, -rtems, and -freebsd so I figured it was wise to only influence those. If anyone disagrees, let me know and I can update this.

Herald added subscribers: frasercrmck, apazos, jocewei and 10 others. · View Herald TranscriptMar 23 2021, 3:41 PM
MaskRay accepted this revision.Mar 23 2021, 4:22 PM
MaskRay added inline comments.
clang/test/CodeGen/mcount.c
17

No need to test both riscv32 and riscv32-elf. They are the same - generic ELF.

This revision is now accepted and ready to land.Mar 23 2021, 4:22 PM
Harbormaster completed remote builds in B95360: Diff 332811.Mar 23 2021, 8:43 PM
nathanchance updated this revision to Diff 333185.Mar 24 2021, 6:00 PM

Drop "riscv32" and "riscv64" from list of checked triples (thanks to Fangrui for catching it with a good explanation).

This revision was landed with ongoing or failed builds.Mar 24 2021, 6:13 PM
Closed by commit rGef58ae86ba77: [RISCV] Fix mcount name (authored by nathanchance). · Explain Why
This revision was automatically updated to reflect the committed changes.
nathanchance added a commit: rGef58ae86ba77: [RISCV] Fix mcount name.
Harbormaster completed remote builds in B95616: Diff 333185.Mar 25 2021, 1:17 AM

Revision Contents

PathSize
clang/
lib/
Basic/
Targets/
6 lines
1 line
test/
CodeGen/
7 lines

Diff 333185

clang/lib/Basic/Targets/OSTargets.h

Show First 20 Lines • Show All 255 Lines • ▼ Show 20 Lines FreeBSDTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
case llvm::Triple::ppcle: case llvm::Triple::ppcle:
case llvm::Triple::ppc64: case llvm::Triple::ppc64:
case llvm::Triple::ppc64le: case llvm::Triple::ppc64le:
this->MCountName = "_mcount"; this->MCountName = "_mcount";
break; break;
case llvm::Triple::arm: case llvm::Triple::arm:
this->MCountName = "__mcount"; this->MCountName = "__mcount";
break; break;
case llvm::Triple::riscv32:
case llvm::Triple::riscv64:
break;
} }
} }
}; };
// GNU/kFreeBSD Target // GNU/kFreeBSD Target
template <typename Target> template <typename Target>
class LLVM_LIBRARY_VISIBILITY KFreeBSDTargetInfo : public OSTargetInfo<Target> { class LLVM_LIBRARY_VISIBILITY KFreeBSDTargetInfo : public OSTargetInfo<Target> {
protected: protected:
▲ Show 20 Lines • Show All 214 Lines • ▼ Show 20 Lines OpenBSDTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
case llvm::Triple::mips64: case llvm::Triple::mips64:
case llvm::Triple::mips64el: case llvm::Triple::mips64el:
case llvm::Triple::ppc: case llvm::Triple::ppc:
case llvm::Triple::ppc64: case llvm::Triple::ppc64:
case llvm::Triple::ppc64le: case llvm::Triple::ppc64le:
case llvm::Triple::sparcv9: case llvm::Triple::sparcv9:
this->MCountName = "_mcount"; this->MCountName = "_mcount";
break; break;
case llvm::Triple::riscv32:
case llvm::Triple::riscv64:
break;
} }
} }
}; };
// PSP Target // PSP Target
template <typename Target> template <typename Target>
class LLVM_LIBRARY_VISIBILITY PSPTargetInfo : public OSTargetInfo<Target> { class LLVM_LIBRARY_VISIBILITY PSPTargetInfo : public OSTargetInfo<Target> {
protected: protected:
▲ Show 20 Lines • Show All 453 Lines • Show Last 20 Lines

clang/lib/Basic/Targets/RISCV.h

Show First 20 Lines • Show All 53 Lines • ▼ Show 20 Lines RISCVTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
: TargetInfo(Triple) { : TargetInfo(Triple) {
LongDoubleWidth = 128; LongDoubleWidth = 128;
LongDoubleAlign = 128; LongDoubleAlign = 128;
LongDoubleFormat = &llvm::APFloat::IEEEquad(); LongDoubleFormat = &llvm::APFloat::IEEEquad();
SuitableAlign = 128; SuitableAlign = 128;
WCharType = SignedInt; WCharType = SignedInt;
WIntType = UnsignedInt; WIntType = UnsignedInt;
HasRISCVVTypes = true; HasRISCVVTypes = true;
MCountName = "_mcount";
} }
bool setCPU(const std::string &Name) override { bool setCPU(const std::string &Name) override {
if (!isValidCPUName(Name)) if (!isValidCPUName(Name))
return false; return false;
CPU = Name; CPU = Name;
return true; return true;
} }
▲ Show 20 Lines • Show All 99 Lines • Show Last 20 Lines

clang/test/CodeGen/mcount.c

// RUN: %clang_cc1 -pg -triple i386-unknown-unknown -emit-llvm -o - %s | FileCheck %s // RUN: %clang_cc1 -pg -triple i386-unknown-unknown -emit-llvm -o - %s | FileCheck %s
// RUN: %clang_cc1 -pg -triple i386-unknown-unknown -emit-llvm -O2 -o - %s | FileCheck %s // RUN: %clang_cc1 -pg -triple i386-unknown-unknown -emit-llvm -O2 -o - %s | FileCheck %s
// RUN: %clang_cc1 -pg -triple powerpc-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple powerpc-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple powerpc64-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple powerpc64-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple powerpc64le-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple powerpc64le-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple i386-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple i386-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple x86_64-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple x86_64-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple arm-netbsd-eabi -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple arm-netbsd-eabi -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple aarch64-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple aarch64-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple mips-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple mips-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple mips-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple mips-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple mipsel-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple mips64-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple mips64-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple mips64el-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple mips64el-unknown-gnu-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple riscv32-elf -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple riscv64-elf -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple riscv32-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
MaskRayUnsubmitted
Not Done
ReplyInline Actions

No need to test both riscv32 and riscv32-elf. They are the same - generic ELF.

MaskRay: No need to test both `riscv32` and `riscv32-elf`. They are the same - generic ELF.
// RUN: %clang_cc1 -pg -triple riscv64-linux -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple riscv64-freebsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple riscv64-openbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple powerpc-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple powerpc-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple powerpc64-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple powerpc64-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple powerpc64le-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple powerpc64le-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple sparc-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple sparc-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -pg -triple sparc64-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s // RUN: %clang_cc1 -pg -triple sparc64-netbsd -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-DOUBLE-PREFIXED,NO-MCOUNT1 %s
// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s -check-prefix=NO-MCOUNT // RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s -check-prefix=NO-MCOUNT
int bar(void) { int bar(void) {
Show All 29 Lines