Skip to content

Test itanium mangling of f16 and f128 - #162151

Open
folkertdev wants to merge 4 commits into
rust-lang:mainfrom
folkertdev:ppc-itanium
Open

Test itanium mangling of f16 and f128#162151
folkertdev wants to merge 4 commits into
rust-lang:mainfrom
folkertdev:ppc-itanium

Conversation

@folkertdev

@folkertdev folkertdev commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

And fix the string that is used for f128 on powerpc targets, where the standard "g" is already taken by __ibm128.

In clang

https://github.com/llvm/llvm-project/blob/bd5b1f58ae58cceab2cadb882cceb1d96f4ad33e/clang/lib/Basic/Targets/PPC.h#L362-L363

  const char *getFloat128Mangling() const override { return "u9__ieee128"; }
  const char *getIbm128Mangling() const override { return "g"; }

and GCC

https://github.com/gcc-mirror/gcc/blob/fc54ab94ad257f9ef43a7287a60c21734a7a288f/gcc/config/rs6000/rs6000.cc#L20795-L20801

  if (SCALAR_FLOAT_TYPE_P (type) && FLOAT128_IBM_P (TYPE_MODE (type)))
    return "g";
  if (SCALAR_FLOAT_TYPE_P (type) && FLOAT128_IEEE_P (TYPE_MODE (type)))
    return "u9__ieee128";

From what i can tell this is not really testable for powerpc at the moment because CFI is not supported on powerpc.

r? tgross35

@folkertdev folkertdev added A-sanitizers Area: Sanitizers for correctness and code quality F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` labels Sep 1, 2026
@rustbot

rustbot commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in tests/codegen-llvm/sanitizer

cc @rcvalle

Some changes occurred in compiler/rustc_sanitizers

cc @rcvalle

@rustbot rustbot added PG-exploit-mitigations Project group: Exploit mitigations 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 Sep 1, 2026
@rustbot

rustbot commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

tgross35 is currently at their maximum review capacity.
They may take a while to respond.

@folkertdev

folkertdev commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

@rcvalle feel free to approve this too if it looks good to you

tgross35
tgross35 reviewed Sep 1, 2026
View reviewed changes
Comment thread compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/encode.rs
Comment on lines +473 to +476
FloatTy::F128 => match tcx.sess.target.arch {
Arch::PowerPC | Arch::PowerPC64 => "u9__ieee128", // "g" is used for __ibm128
_ => "g",
},

@tgross35 tgross35 Sep 1, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later in this file is

rust/compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/encode.rs

Lines 555 to 558 in a433023

let builtin_types = [
"v", "w", "b", "c", "a", "h", "s", "t", "i", "j", "l", "m", "x", "y", "n", "o",
"f", "d", "e", "g", "z", "Dh",
];
which I guess also needs to be updated?

View changes since the review

@folkertdev folkertdev Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, fixed

Comment thread tests/codegen-llvm/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-primitive-types.rs

@tgross35 tgross35 Sep 1, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is minicore now, could it get a ppc test?

View changes since the review

@folkertdev folkertdev Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because CFI is not supported on powerpc64.

@tgross35 tgross35 Sep 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, missed that in the description. Guess the addition is dead code then, but doesn't hurt to fix if it winds up reused elsewhere.

@folkertdev
fix itanium mangling of f128 on powerpc
1bd13e5 
because "g" is used for __ibm128, __float128 gets a custom name
tgross35
tgross35 approved these changes Sep 1, 2026
View reviewed changes

@tgross35 tgross35 left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, feel free to merge unless you're specifically waiting on Ramon

View changes since this review

Comment thread tests/codegen-llvm/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-primitive-types.rs

@tgross35 tgross35 Sep 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, missed that in the description. Guess the addition is dead code then, but doesn't hurt to fix if it winds up reused elsewhere.

@folkertdev

folkertdev commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

@bors r=tgross35

@rust-bors

rust-bors Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 1bd13e5 has been approved by tgross35

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgross35 tgross35 tgross35 approved these changes

Assignees

@tgross35 tgross35

Labels

A-sanitizers Area: Sanitizers for correctness and code quality F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@folkertdev @rustbot @tgross35