Repository navigation
cfi: mangle f128 as e rather than g on platforms without _Float128 - #163193
Conversation
|
|
This comment has been minimized.
This comment has been minimized.
78318cd to
52e6e34
Compare
|
Does Clang encode |
| // `f128` is long double (`e`) only where that's 128-bit; Apple/Windows aarch64 | ||
| // use a 64-bit `long double` and fall through to `g` (`_Float128`, absent there). | ||
| Arch::AArch64 | ||
| if !tcx.sess.target.is_like_darwin && !tcx.sess.target.is_like_windows => | ||
| { | ||
| "e" | ||
| } |
There was a problem hiding this comment.
This logic should probably go in impl Target as is_long_double_f128 with a FIXME to change to long_double_type(...) -> rustc_abi::Float, since I know we'll eventually be reusing this logic.
There was a problem hiding this comment.
Applied. Agreed with your opinion, this logic might reused by PAC.
|
52e6e34 to
6c97266
Compare
| pub fn is_long_double_f128(&self) -> bool { | ||
| self.arch == Arch::AArch64 && !self.is_like_darwin && !self.is_like_windows | ||
| } |
There was a problem hiding this comment.
match self.arch {
Arch::AArch64 => !self.is_like_darwin && !self.is_like_windows,
Arch::RiscV32 => true,
Arch::RiscV64 => true,
Arch::S390x => true,
_ => false,
}Referencing my research at #140417.
There was a problem hiding this comment.
Thank you very much, I have reviewed the implementation in #140417. I identified two issues: first, support for loongarch64 was not taken into account; second, regarding s390x, although it should use 'e', it was explicitly overridden to 'g'. If the current function were used, it would incorrectly emit 'e', so it needs to be explicitly excluded. (Additionally, while s390x does not currently support CFI, I believe this case should still be considered, given that the current function might be reused for PAC in the future.)
There was a problem hiding this comment.
Sorry to do this, but would you mind deleting this function and going back to what you had? I thought this always corresponded to long double === _Float128, but if that's not the case then I think it's better to just inline.
There was a problem hiding this comment.
Done, it's inline now.
| @@ -473,6 +472,7 @@ pub(crate) fn encode_ty<'tcx>( | |||
| FloatTy::F64 => "d", | |||
| FloatTy::F128 => match tcx.sess.target.arch { | |||
| Arch::PowerPC | Arch::PowerPC64 => "u9__ieee128", // "g" is used for __ibm128 | |||
| _ if tcx.sess.target.is_long_double_f128() => "e", | |||
There was a problem hiding this comment.
Please add a small note that Clang is more friendly toward long double, and not all platforms support _Float128
|
Reminder, once the PR becomes ready for a review, use |
a2dcc03 to
856a041
Compare
856a041 to
7dd5566
Compare
| @@ -473,6 +472,14 @@ pub(crate) fn encode_ty<'tcx>( | |||
| FloatTy::F64 => "d", | |||
| FloatTy::F128 => match tcx.sess.target.arch { | |||
| Arch::PowerPC | Arch::PowerPC64 => "u9__ieee128", // "g" is used for __ibm128 | |||
| Arch::S390x => "g", // On s390x, `f128` corresponds to `long double`, but the psABI specifies `g` as its mangling code. | |||
There was a problem hiding this comment.
Drop this one since it will hit the fallback, then LGTM
There was a problem hiding this comment.
Sure. Already deleted.
7dd5566 to
200fefd
Compare
|
@rustbot ready |
f128 on aarch64 Linux incorrectly sends gf128 as e rather than g on platforms without _Float128
…oss35 cfi: mangle `f128` as `e` rather than `g` on platforms without `_Float128` On the Linux platform, only `x86` and `x86_64` support `Float128`. (see https://github.com/llvm/llvm-project/blob/bd5b1f58ae58cceab2cadb882cceb1d96f4ad33e/clang/lib/Basic/Targets/OSTargets.h#L431) Therefore, for aarch64, the `f128` type corresponds exclusively to `long double`. In addition, `aarch64` does not overload `getLongDoubleMangling` and `getFloat128Mangling`.(see https://github.com/llvm/llvm-project/blob/bd5b1f58ae58cceab2cadb882cceb1d96f4ad33e/clang/include/clang/Basic/TargetInfo.h#L822) Therefore, `aarch64` on Linux should send `e` instead of `g`. For Windows and Apple `aarch64`, the `long double` type is 64-bit, so these targets need to be excluded. The corresponding tests are added. LLM disclosure: This commit is entirely handwritten. r? @tgross35
…oss35 cfi: mangle `f128` as `e` rather than `g` on platforms without `_Float128` On the Linux platform, only `x86` and `x86_64` support `Float128`. (see https://github.com/llvm/llvm-project/blob/bd5b1f58ae58cceab2cadb882cceb1d96f4ad33e/clang/lib/Basic/Targets/OSTargets.h#L431) Therefore, for aarch64, the `f128` type corresponds exclusively to `long double`. In addition, `aarch64` does not overload `getLongDoubleMangling` and `getFloat128Mangling`.(see https://github.com/llvm/llvm-project/blob/bd5b1f58ae58cceab2cadb882cceb1d96f4ad33e/clang/include/clang/Basic/TargetInfo.h#L822) Therefore, `aarch64` on Linux should send `e` instead of `g`. For Windows and Apple `aarch64`, the `long double` type is 64-bit, so these targets need to be excluded. The corresponding tests are added. LLM disclosure: This commit is entirely handwritten. r? @tgross35
…uwer Rollup of 14 pull requests Successful merges: - #162652 (Syntactically reject leading parenthesized precise capturing lists in bare trait object types (`(use<…>)+`)) - #163337 (MIR move elimination [3/6]: PreciseLiveness) - #163954 (fix(bootstrap/darwin): fix rpath for distributed LLD) - #163193 (cfi: mangle `f128` as `e` rather than `g` on platforms without `_Float128`) - #163634 (move overflow lint computation into decorator) - #163666 (Updates the expect message library/core/src/time.rs) - #163727 (rigid aliases to non-rigid for fully normalized check) - #163745 (replace `fully_monomorphized` with `cx.typing_env()`) - #163912 (Fix debug assert failure in `note_obligation_cause_code_inner`) - #163972 (const-eval: ICE when we hit a non-const fn) - #164000 (When mentioning that closure doesn't implement trait, point at closure) - #164007 ([rustdoc] Prefer local paths over remote ones when foreign item is locally reexported) - #164017 (cg_llvm: Avoid some explicit casts to `*const c_char`) - #164025 (Less `CanonicalVarValues`)
…oss35 cfi: mangle `f128` as `e` rather than `g` on platforms without `_Float128` On the Linux platform, only `x86` and `x86_64` support `Float128`. (see https://github.com/llvm/llvm-project/blob/bd5b1f58ae58cceab2cadb882cceb1d96f4ad33e/clang/lib/Basic/Targets/OSTargets.h#L431) Therefore, for aarch64, the `f128` type corresponds exclusively to `long double`. In addition, `aarch64` does not overload `getLongDoubleMangling` and `getFloat128Mangling`.(see https://github.com/llvm/llvm-project/blob/bd5b1f58ae58cceab2cadb882cceb1d96f4ad33e/clang/include/clang/Basic/TargetInfo.h#L822) Therefore, `aarch64` on Linux should send `e` instead of `g`. For Windows and Apple `aarch64`, the `long double` type is 64-bit, so these targets need to be excluded. The corresponding tests are added. LLM disclosure: This commit is entirely handwritten. r? @tgross35
…uwer Rollup of 14 pull requests Successful merges: - #162652 (Syntactically reject leading parenthesized precise capturing lists in bare trait object types (`(use<…>)+`)) - #163337 (MIR move elimination [3/6]: PreciseLiveness) - #163954 (fix(bootstrap/darwin): fix rpath for distributed LLD) - #163193 (cfi: mangle `f128` as `e` rather than `g` on platforms without `_Float128`) - #163634 (move overflow lint computation into decorator) - #163666 (Updates the expect message library/core/src/time.rs) - #163727 (rigid aliases to non-rigid for fully normalized check) - #163745 (replace `fully_monomorphized` with `cx.typing_env()`) - #163912 (Fix debug assert failure in `note_obligation_cause_code_inner`) - #163950 (don't treat inherited opaques as defining) - #164000 (When mentioning that closure doesn't implement trait, point at closure) - #164007 ([rustdoc] Prefer local paths over remote ones when foreign item is locally reexported) - #164017 (cg_llvm: Avoid some explicit casts to `*const c_char`) - #164025 (Less `CanonicalVarValues`)
…uwer Rollup of 24 pull requests Successful merges: - #161998 ( Support type-relative assoc item paths in generic param defaults & const param types) - #162106 (Helpful suggestions for incorrect address-of mutability (2)) - #162652 (Syntactically reject leading parenthesized precise capturing lists in bare trait object types (`(use<…>)+`)) - #163337 (MIR move elimination [3/6]: PreciseLiveness) - #163938 (-Zassumptions-on-binders: rewrite alias outlives constraints more goodly) - #163939 (Better debug impls for some assumptions on binders types) - #163954 (fix(bootstrap/darwin): fix rpath for distributed LLD) - #163956 (Pass the unremapped path to the `rustc` invocation for doctests) - #164042 (Allow testing cg-gcc on any target) - #162443 (Do not retain `Normalization` goal errors in nested goals for `BestObligationVisitor:: non_trivial_candidates `) - #162908 (Fix - const parameters rejected when identical) - #163193 (cfi: mangle `f128` as `e` rather than `g` on platforms without `_Float128`) - #163634 (move overflow lint computation into decorator) - #163666 (Updates the expect message library/core/src/time.rs) - #163727 (rigid aliases to non-rigid for fully normalized check) - #163745 (replace `fully_monomorphized` with `cx.typing_env()`) - #163912 (Fix debug assert failure in `note_obligation_cause_code_inner`) - #163950 (don't treat inherited opaques as defining) - #163972 (const-eval: ICE when we hit a non-const fn) - #164000 (When mentioning that closure doesn't implement trait, point at closure) - #164007 ([rustdoc] Prefer local paths over remote ones when foreign item is locally reexported) - #164008 (properly ignore the current goal's usages) - #164017 (cg_llvm: Avoid some explicit casts to `*const c_char`) - #164025 (Less `CanonicalVarValues`)
Rollup merge of #163193 - cezarbbb:pac-f128-aarch64-e, r=tgross35 cfi: mangle `f128` as `e` rather than `g` on platforms without `_Float128` On the Linux platform, only `x86` and `x86_64` support `Float128`. (see https://github.com/llvm/llvm-project/blob/bd5b1f58ae58cceab2cadb882cceb1d96f4ad33e/clang/lib/Basic/Targets/OSTargets.h#L431) Therefore, for aarch64, the `f128` type corresponds exclusively to `long double`. In addition, `aarch64` does not overload `getLongDoubleMangling` and `getFloat128Mangling`.(see https://github.com/llvm/llvm-project/blob/bd5b1f58ae58cceab2cadb882cceb1d96f4ad33e/clang/include/clang/Basic/TargetInfo.h#L822) Therefore, `aarch64` on Linux should send `e` instead of `g`. For Windows and Apple `aarch64`, the `long double` type is 64-bit, so these targets need to be excluded. The corresponding tests are added. LLM disclosure: This commit is entirely handwritten. r? @tgross35
On the Linux platform, only
x86andx86_64supportFloat128. (see https://github.com/llvm/llvm-project/blob/bd5b1f58ae58cceab2cadb882cceb1d96f4ad33e/clang/lib/Basic/Targets/OSTargets.h#L431)Therefore, for aarch64, the
f128type corresponds exclusively tolong double.In addition,
aarch64does not overloadgetLongDoubleManglingandgetFloat128Mangling.(see https://github.com/llvm/llvm-project/blob/bd5b1f58ae58cceab2cadb882cceb1d96f4ad33e/clang/include/clang/Basic/TargetInfo.h#L822)Therefore,
aarch64on Linux should sendeinstead ofg.For Windows and Apple
aarch64, thelong doubletype is 64-bit, so these targets need to be excluded.The corresponding tests are added.
LLM disclosure: This commit is entirely handwritten.
r? @tgross35