Add Natvis visualiser and debuginfo tests for `f16` #127001

beetrees · 2024-06-26T18:23:42Z

To render f16s in debuggers on MSVC targets, this PR changes the compiler to output f16s as struct f16 { bits: u16 }, and includes a Natvis visualiser that manually converts the f16's bits to a float which is can then be displayed by debuggers. gdb, lldb and cdb tests are also included for f16 .

f16/f128 MSVC debug info issue: #121837
Tracking issue: #116909

rustbot · 2024-06-26T18:23:50Z

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

beetrees · 2024-06-26T20:08:27Z

@rustbot label +F-f16_and_f128

tgross35 · 2024-06-27T11:17:15Z

Cc @MaulingMonkey since you suggested what I copied to #121837

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

tgross35 · 2024-06-27T19:36:12Z

@ChrisDenton do you handle the natvis stuff? I seem to remember there not being too many people who were able to review it (though this seems pretty understandable, the math part in intrinsic.natvis looks correct to me)

ChrisDenton · 2024-06-27T19:39:44Z

No, sorry. While I have a little familiarity with the format, I wouldn't trust myself to review. Try @michaelwoerister maybe?

michaelwoerister · 2024-06-27T19:53:42Z

r? @michaelwoerister
I'll take a look next week.

michaelwoerister · 2024-06-27T19:56:41Z

C# seems to have something like f16, so maybe we are lucky:
https://devblogs.microsoft.com/dotnet/introducing-the-half-type/

michaelwoerister

This looks great, thanks @beetrees!
I have only some minor comments.

michaelwoerister · 2024-07-04T13:56:12Z

tests/debuginfo/f16-natvis.rs

@@ -0,0 +1,56 @@
+//@ compile-flags: -g


Can you add //@ only-windows so that we ignore the test on other platforms?

I've added //@ only-msvc as cpp_like_debuginfo is only used on MSVC targets.

michaelwoerister · 2024-07-04T14:02:02Z

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

            ty::FloatTy::F32 => "float",
            ty::FloatTy::F64 => "double",
            ty::FloatTy::F128 => "fp128",
        }
    }
 }

+fn build_cpp_f16_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) -> DINodeCreationResult<'ll> {


Please add a comment explaining why we do this.

michaelwoerister · 2024-07-05T08:24:03Z

Thanks for the updates, @beetrees!

Excluding from rollups since debuginfo tests often fail on the first try in CI.
@bors r+ rollup=never

bors · 2024-07-05T08:24:06Z

📌 Commit b701222 has been approved by michaelwoerister

It is now in the queue for this repository.

bors · 2024-07-05T08:34:50Z

⌛ Testing commit b701222 with merge 66fe6dd...

…rister Add Natvis visualiser and debuginfo tests for `f16` To render `f16`s in debuggers on MSVC targets, this PR changes the compiler to output `f16`s as `struct f16 { bits: u16 }`, and includes a Natvis visualiser that manually converts the `f16`'s bits to a `float` which is can then be displayed by debuggers. `gdb`, `lldb` and `cdb` tests are also included for `f16` . `f16`/`f128` MSVC debug info issue: rust-lang#121837 Tracking issue: rust-lang#116909

bors · 2024-07-05T09:09:09Z

💔 Test failed - checks-actions

michaelwoerister · 2024-07-05T09:22:12Z

tests/debuginfo/basic-types-mut-globals.rs

 // gdbg-command:print 'basic_types_mut_globals'::F64
 // gdbr-command:print F64
-// gdb-check:$28 = 9.25
+// gdb-check:$30 = 9.25

 #![allow(unused_variables)]
 #![feature(omit_gdb_pretty_printer_section)]


Needs a #![feature(f16)].

If you're up for it you can try to remove the //@ ignore-gdb directive at the top, since the very similar basic-types-globals.rs test seems to work just fine.

I've added the missing #![feature(f16)]s, and removed the //@ ignore-gdb (the test passes locally).

…rister Add Natvis visualiser and debuginfo tests for `f16` To render `f16`s in debuggers on MSVC targets, this PR changes the compiler to output `f16`s as `struct f16 { bits: u16 }`, and includes a Natvis visualiser that manually converts the `f16`'s bits to a `float` which is can then be displayed by debuggers. `gdb`, `lldb` and `cdb` tests are also included for `f16` . `f16`/`f128` MSVC debug info issue: rust-lang#121837 Tracking issue: rust-lang#116909

bors · 2024-07-08T23:01:44Z

💔 Test failed - checks-actions

compiler-errors · 2024-07-08T23:46:49Z

@bors retry

bors · 2024-07-09T01:23:58Z

⌛ Testing commit 4252842 with merge 585ffed...

…rister Add Natvis visualiser and debuginfo tests for `f16` To render `f16`s in debuggers on MSVC targets, this PR changes the compiler to output `f16`s as `struct f16 { bits: u16 }`, and includes a Natvis visualiser that manually converts the `f16`'s bits to a `float` which is can then be displayed by debuggers. `gdb`, `lldb` and `cdb` tests are also included for `f16` . `f16`/`f128` MSVC debug info issue: rust-lang#121837 Tracking issue: rust-lang#116909

bors · 2024-07-09T02:16:31Z

💔 Test failed - checks-actions

beetrees · 2024-07-09T02:51:25Z

I've fixed the typos in the test that caused the failure.

michaelwoerister · 2024-07-09T07:37:00Z

@bors r+

bors · 2024-07-09T07:37:02Z

📌 Commit b058de9 has been approved by michaelwoerister

It is now in the queue for this repository.

bors · 2024-07-09T09:07:46Z

⌛ Testing commit b058de9 with merge 8672b2b...

bors · 2024-07-09T11:22:06Z

☀️ Test successful - checks-actions
Approved by: michaelwoerister
Pushing 8672b2b to master...

rust-timer · 2024-07-09T13:53:04Z

Finished benchmarking commit (8672b2b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary -4.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.7%	[-4.7%, -4.7%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (primary -3.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.5%	[-3.5%, -3.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.5%	[-3.5%, -3.5%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 702.485s -> 703.117s (0.09%)
Artifact size: 328.89 MiB -> 328.74 MiB (-0.05%)

rust-lang/rust#127001 (comment)

rustbot assigned estebank Jun 26, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 26, 2024

beetrees force-pushed the f16-debuginfo branch from 197a29c to 64d6157 Compare June 26, 2024 19:47

beetrees marked this pull request as ready for review June 26, 2024 19:47

rustbot added the F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` label Jun 26, 2024

tgross35 reviewed Jun 27, 2024

View reviewed changes

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs Outdated Show resolved Hide resolved

beetrees force-pushed the f16-debuginfo branch from 64d6157 to 0a87558 Compare June 27, 2024 15:05

rustbot assigned michaelwoerister and unassigned estebank Jun 27, 2024

michaelwoerister approved these changes Jul 4, 2024

View reviewed changes

beetrees force-pushed the f16-debuginfo branch from 0a87558 to b701222 Compare July 4, 2024 15:58

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 5, 2024

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 5, 2024

michaelwoerister reviewed Jul 5, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 8, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 8, 2024

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 9, 2024

Add Natvis visualiser and debuginfo tests for f16

b058de9

beetrees force-pushed the f16-debuginfo branch from 4252842 to b058de9 Compare July 9, 2024 02:50

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 9, 2024

bors merged commit 8672b2b into rust-lang:master Jul 9, 2024

rustbot added this to the 1.81.0 milestone Jul 9, 2024

beetrees deleted the f16-debuginfo branch July 9, 2024 14:45

tgross35 mentioned this pull request Jul 10, 2024

Tracking Issue for f16 and f128 float types #116909

Open

92 tasks

Dajamante added a commit to ferrocene/ferrocene that referenced this pull request Jul 25, 2024

Disabled test not passing CI

8c26854

rust-lang/rust#127001 (comment)

Add Natvis visualiser and debuginfo tests for f16 #127001

Add Natvis visualiser and debuginfo tests for f16 #127001

Uh oh!

Conversation

beetrees commented Jun 26, 2024

Uh oh!

rustbot commented Jun 26, 2024

Uh oh!

beetrees commented Jun 26, 2024

Uh oh!

tgross35 commented Jun 27, 2024

Uh oh!

Uh oh!

tgross35 commented Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChrisDenton commented Jun 27, 2024

Uh oh!

michaelwoerister commented Jun 27, 2024

Uh oh!

michaelwoerister commented Jun 27, 2024

Uh oh!

michaelwoerister left a comment

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!

michaelwoerister commented Jul 5, 2024

Uh oh!

bors commented Jul 5, 2024

Uh oh!

bors commented Jul 5, 2024

Uh oh!

This comment has been minimized.

bors commented Jul 5, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

bors commented Jul 8, 2024

Uh oh!

compiler-errors commented Jul 8, 2024

Uh oh!

bors commented Jul 9, 2024

Uh oh!

This comment has been minimized.

bors commented Jul 9, 2024

Uh oh!

beetrees commented Jul 9, 2024

Uh oh!

michaelwoerister commented Jul 9, 2024

Uh oh!

bors commented Jul 9, 2024

Uh oh!

bors commented Jul 9, 2024

Uh oh!

bors commented Jul 9, 2024

Uh oh!

rust-timer commented Jul 9, 2024

Overall result: no relevant changes - no action needed

Uh oh!

Uh oh!

Add Natvis visualiser and debuginfo tests for `f16` #127001

Add Natvis visualiser and debuginfo tests for `f16` #127001

tgross35 commented Jun 27, 2024 •

edited

Loading