When writing LLVM IR output demangled fn name in comments #42971

stepancheg · 2017-06-29T18:33:18Z

--emit=llvm-ir looks like this now:

; <alloc::vec::Vec<T> as core::ops::index::IndexMut<core::ops::range::RangeFull>>::index_mut
; Function Attrs: inlinehint uwtable
define internal { i8*, i64 } @"_ZN106_$LT$alloc..vec..Vec$LT$T$GT$$u20$as$u20$core..ops..index..IndexMut$LT$core..ops..range..RangeFull$GT$$GT$9index_mut17h7f7b576609f30262E"(%"alloc::vec::Vec<u8>"* dereferenceable(24)) unnamed_addr #0 {
start:
  ...

cc rust-lang/rust-playground#15

rust-highfive · 2017-06-29T18:33:29Z

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

stepancheg · 2017-06-29T22:26:06Z

Updated PR with call and invoke instruction comments:

; std::collections::hash::table::calculate_offsets
; Function Attrs: inlinehint uwtable
define internal void @_ZN3std11collections4hash5table17calculate_offsets17hccd439e3af053e99E({ i64, [0 x i8], i64, [0 x i8], i8, [7 x i8] }* noalias nocapture sret dereferenceable(24), i64, i64, i64) unnamed_addr #0 {
start:
  %tmp_ret = alloca { i64, [0 x i8], i8, [7 x i8] }
; call std::collections::hash::table::round_up_to_next
  %4 = call i64 @_ZN3std11collections4hash5table16round_up_to_next17hb5dd1061b779aff5E(i64 %1, i64 %3)
  ...

stepancheg · 2017-06-29T22:30:24Z

Sample source code and output: gist.

nagisa · 2017-06-30T15:33:35Z

src/libstd/lib.rs

+pub mod demangle_for_rustc {
+    pub use sys_common::backtrace::demangle;
+    pub use sys_common::backtrace::PrintFormat;
+}


IMO it is a shame that we’re exposing more unstable stuff from libstd. Would it be possible to extract the functionality into a crate and make both libstd and librustc_trans use that?

Yes, I'll try to do that now. What should be crate name, librust_demange?

Well there’s technically a rustc-demangle already, although I do not know how easy/hard it is to integrate an external crate into libstd. I know there are a few already integrated in other places such as librustc_trans, could take a look there.

@alexcrichton any suggestions on that issue?

nagisa · 2017-06-30T15:35:49Z

src/rustllvm/PassWrapper.cpp

+  // return empty string if demangle failed
+  // or if name does not need to be demangled
+  StringRef CallDemangle(StringRef name) {
+    if (!Demangle) {


Could the constructor use a default no-op demangle callback instead?

EDIT: And yes, it seems fairly plausible for somebody to pass in NULL for this.

I can just remove the comment. No-op demangle would be overengineering IMHO.

nagisa · 2017-06-30T15:38:24Z

src/rustllvm/PassWrapper.cpp

+class RustAssemblyAnnotationWriter : public AssemblyAnnotationWriter {
+  DemangleFn Demangle;
+  static const size_t BufCap = 500;
+  char Buf[BufCap];


Could the output buffer size derived from the input symbol instead? Something like a std::vector that only ever expands when it sees an input symbol longer than its current output buffer.

OK, I'll do that.

nagisa · 2017-06-30T15:41:15Z

src/rustllvm/PassWrapper.cpp

+                            formatted_raw_ostream &OS) override {
+    const char *Name;
+    const Value *Value;
+    if (const CallInst *CI = dyn_cast<CallInst>(I)) {


Some other cases where a demangle could be relevant is taking the address of the function.

That would be store %place, @function.

Maybe it is worth to always look at the instruction operands and output a demangled version if there’s a function operand?

I'll keep printing only invoke and call for now (which covers 99% of cases), and probably do more printing in another PR, if that's OK with you.

Yeah, that’s fine.

nagisa · 2017-06-30T15:42:02Z

src/rustllvm/PassWrapper.cpp

+  bool runOnModule(Module &M) override {
+    RustAssemblyAnnotationWriter AW(Demangle);
+
+    // Copy-paste of PrintModulePass::run


Remove the comments?

nagisa · 2017-06-30T15:43:51Z

r? @nagisa

stepancheg · 2017-06-30T19:57:14Z

Updated the PR.

Removed comments, replace fixed size buf with std::vector.

librustc_trans now uses rustc-demangle from crates, but std still contains private implementation of demangle (because I don't know what could happen if std will depend on crates from cargo, removal of demangle from std can be done later).

nagisa · 2017-06-30T20:00:33Z

Awesome, thanks!

@bors r+

bors · 2017-06-30T20:00:34Z

📌 Commit 8f6ac5b has been approved by nagisa

`--emit=llvm-ir` looks like this now: ``` ; <alloc::vec::Vec<T> as core::ops::index::IndexMut<core::ops::range::RangeFull>>::index_mut ; Function Attrs: inlinehint uwtable define internal { i8*, i64 } @"_ZN106_$LT$alloc..vec..Vec$LT$T$GT$$u20$as$u20$core..ops..index..IndexMut$LT$core..ops..range..RangeFull$GT$$GT$9index_mut17h7f7b576609f30262E"(%"alloc::vec::Vec<u8>"* dereferenceable(24)) unnamed_addr #0 { start: ... ``` cc rust-lang/rust-playground#15

stepancheg · 2017-07-01T00:43:20Z

After changing to rustc-demangle, I overlooked that comments now contain function hashes, which they shouldn't.

I've pushed a fix to the branch. The only change is this:

diff --git a/src/librustc_trans/back/write.rs b/src/librustc_trans/back/write.rs
index cfb5045afe..562d717115 100644
--- a/src/librustc_trans/back/write.rs
+++ b/src/librustc_trans/back/write.rs
@@ -537,7 +537,7 @@ unsafe fn optimize_and_codegen(cgcx: &CodegenContext,
                     Err(_) => return 0,
                 };

-                if let Err(_) = write!(cursor, "{}", demangled) {
+                if let Err(_) = write!(cursor, "{:#}", demangled) {
                     // Possible only if provided buffer is not big enough
                     return 0;
                 }

Someone do r+ again please.

stepancheg · 2017-07-01T01:11:24Z

@bors r-

(Can I do that?)

bors · 2017-07-01T01:11:24Z

@stepancheg: 🔑 Insufficient privileges: Not in reviewers

Mark-Simulacrum · 2017-07-01T02:05:42Z

@bors r=nagisa

bors · 2017-07-01T02:05:42Z

📌 Commit b62bdaa has been approved by nagisa

bors · 2017-07-01T05:52:15Z

⌛ Testing commit b62bdaa with merge d41b791...

When writing LLVM IR output demangled fn name in comments `--emit=llvm-ir` looks like this now: ``` ; <alloc::vec::Vec<T> as core::ops::index::IndexMut<core::ops::range::RangeFull>>::index_mut ; Function Attrs: inlinehint uwtable define internal { i8*, i64 } @"_ZN106_$LT$alloc..vec..Vec$LT$T$GT$$u20$as$u20$core..ops..index..IndexMut$LT$core..ops..range..RangeFull$GT$$GT$9index_mut17h7f7b576609f30262E"(%"alloc::vec::Vec<u8>"* dereferenceable(24)) unnamed_addr #0 { start: ... ``` cc rust-lang/rust-playground#15

bors · 2017-07-01T08:12:40Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nagisa
Pushing d41b791 to master...

With this patch `--emit=asm` looks like this: ``` ; <alloc::vec::Vec<T> as core::ops::index::IndexMut<core::ops::range::RangeFull>>::index_mut __ZN106_$LT$alloc..vec..Vec$LT$T$GT$$u20$as$u20$core..ops..index..IndexMut$LT$core..ops..range..RangeFull$GT$$GT$9index_mut17h8ecb05aacf724133E: ... Lcfi2: .cfi_def_cfa_register %rbp subq $16, %rsp ; <alloc::vec::Vec<T> as core::ops::deref::DerefMut>::deref_mut callq __ZN71_$LT$alloc..vec..Vec$LT$T$GT$$u20$as$u20$core..ops..deref..DerefMut$GT$9deref_mut17heae350c695265f00E ``` Unlike writing LLVM IR, it is hard to plug into LLVM to output comments without duplicating large amount of LLVM code (because LLVM doesn't have hooks, or at least I haven't found them). So it is implemented by post-processing assembly text. cc rust-lang/rust-playground#15 cc rust-lang#42971 r? nagisa

rust-highfive assigned alexcrichton Jun 29, 2017

stepancheg force-pushed the ir-demangle branch 3 times, most recently from fc5cd2c to 6404a90 Compare June 29, 2017 22:24

stepancheg force-pushed the ir-demangle branch from 6404a90 to 3e39e5b Compare June 29, 2017 22:59

nagisa reviewed Jun 30, 2017

View reviewed changes

rust-highfive assigned nagisa and unassigned alexcrichton Jun 30, 2017

stepancheg force-pushed the ir-demangle branch from 3e39e5b to 8f6ac5b Compare June 30, 2017 19:52

shepmaster added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 30, 2017

stepancheg force-pushed the ir-demangle branch from 8f6ac5b to b62bdaa Compare July 1, 2017 00:43

bors merged commit b62bdaa into rust-lang:master Jul 1, 2017

bors mentioned this pull request Jul 1, 2017

rustc: Implement the #[global_allocator] attribute #42727

Merged

stepancheg mentioned this pull request Jul 1, 2017

When writing assembly output demangled fn name in comments #43007

Closed

mingodad mentioned this pull request Jul 13, 2021

SQLite extension vovkos/jancy#3

Closed

When writing LLVM IR output demangled fn name in comments #42971

When writing LLVM IR output demangled fn name in comments #42971

Uh oh!

Conversation

stepancheg commented Jun 29, 2017

Uh oh!

rust-highfive commented Jun 29, 2017

Uh oh!

stepancheg commented Jun 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stepancheg commented Jun 29, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nagisa Jun 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nagisa commented Jun 30, 2017

Uh oh!

stepancheg commented Jun 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nagisa commented Jun 30, 2017

Uh oh!

bors commented Jun 30, 2017

Uh oh!

stepancheg commented Jul 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stepancheg commented Jul 1, 2017

Uh oh!

bors commented Jul 1, 2017

Uh oh!

Mark-Simulacrum commented Jul 1, 2017

Uh oh!

bors commented Jul 1, 2017

Uh oh!

bors commented Jul 1, 2017

Uh oh!

bors commented Jul 1, 2017

Uh oh!

Uh oh!

stepancheg commented Jun 29, 2017 •

edited

Loading

nagisa Jun 30, 2017 •

edited

Loading

stepancheg commented Jun 30, 2017 •

edited

Loading

stepancheg commented Jul 1, 2017 •

edited

Loading