Page MenuHomePhabricator

RalfJung (Ralf)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 1 2019, 9:58 AM (216 w, 2 d)

Recent Activity

Feb 10 2023

RalfJung added a comment to D143074: [LangRef] improve documentation of SNaN in the default FP environment.

My worry is: Does having such an indeterminate output value, combined with other optimization passes, trigger unbounded UB from the system as-a-whole? E.g., because we can duplicate and coalesce FP math instructions, and make a different optimization decision for each duplicated instance separately, a single fadd with an sNaN input could appear to be a qNaN to some of its uses and an sNaN for others. Which then as discussed changes the results of finite values from FP computations too. Could that cause problems in downstream optimization passes?

Feb 10 2023, 1:44 AM · Restricted Project, Restricted Project

Feb 9 2023

RalfJung added a comment to D143074: [LangRef] improve documentation of SNaN in the default FP environment.

because, from what I understand, before IEEE 754 specified how to encode quietness for NaNs, MIPS (and PA-RISC) arbitrarily chose the opposite encoding to what IEEE 754-2008 specifies, so LLVM generating quiet NaNs following IEEE 754-2008 produces NaNs that are actually signalling NaNs for old MIPS. MIPS later added a mode bit allowing swapping its interpretation of signalling/quiet NaNs to fall in line with the IEEE 754-2008 spec -- new MIPS has that set to IEEE 754-2008 mode.

Feb 9 2023, 2:47 AM · Restricted Project, Restricted Project
RalfJung added a comment to D143074: [LangRef] improve documentation of SNaN in the default FP environment.

Then, LLVM is broken on old MIPS. There's just no way it's okay to spuriously introduce sNaNs when the original program didn't contain sNaNs in the first place. It results in incorrect results, without the original user code breaking any assumptions.

Feb 9 2023, 2:29 AM · Restricted Project, Restricted Project

Feb 5 2023

RalfJung added a comment to D143074: [LangRef] improve documentation of SNaN in the default FP environment.

Regarding what LLVM actually does, I think it would be more accurate to say that floating-point IR instructions are lowered to the closest target-specific counterpart, but optimizations assume that the lowering conforms to IEEE 754 semantics

Feb 5 2023, 9:34 AM · Restricted Project, Restricted Project

Feb 4 2023

RalfJung added a comment to D143074: [LangRef] improve documentation of SNaN in the default FP environment.

Floating-point math operations assume that all NaNs are quiet.

Feb 4 2023, 6:50 AM · Restricted Project, Restricted Project

Feb 1 2023

RalfJung added a comment to D143074: [LangRef] improve documentation of SNaN in the default FP environment.

Okay, so the rules would be something like: when a floating point operation outputs a NaN, that is

  • either any of its input NaNs (even if that is signaling, which violates IEEE-754)
  • or an arbitrary quiet NaN
Feb 1 2023, 8:42 AM · Restricted Project, Restricted Project
RalfJung added a comment to D143074: [LangRef] improve documentation of SNaN in the default FP environment.

No, arithmetic operations cannot produce signaling nans. Signaling nans only come from initialization.

Feb 1 2023, 8:32 AM · Restricted Project, Restricted Project
RalfJung added a comment to D143074: [LangRef] improve documentation of SNaN in the default FP environment.

"undefined" is a dangerous word. I hope you didn't mean that it is literally undef.

Feb 1 2023, 7:58 AM · Restricted Project, Restricted Project

Jan 31 2023

RalfJung added a comment to D142097: [InstCombine] Don't replace unused `atomicrmw xchg` with `atomic store`.

I agree this optimization should not be performed. Also see https://github.com/llvm/llvm-project/issues/60418 for a counterexample.

Jan 31 2023, 6:25 AM · Restricted Project, Restricted Project

Jan 9 2023

RalfJung added inline comments to D141277: [InstCombine] Don't optimize idempotent `atomicrmw <op>, 0` into `load atomic`.
Jan 9 2023, 11:50 PM · Restricted Project, Restricted Project
RalfJung added inline comments to D141277: [InstCombine] Don't optimize idempotent `atomicrmw <op>, 0` into `load atomic`.
Jan 9 2023, 8:43 AM · Restricted Project, Restricted Project

Dec 28 2022

RalfJung added inline comments to D104268: [ptr_provenance] Introduce optional ptr_provenance operand to load/store.
Dec 28 2022, 8:14 AM · Restricted Project, Restricted Project

Dec 21 2022

RalfJung added a comment to D69542: Full Restrict Support - single patch.

I see, thanks. I guess this summary patch is then the best place for those comments still.

Dec 21 2022, 1:43 PM · Restricted Project, Restricted Project
RalfJung added inline comments to D104268: [ptr_provenance] Introduce optional ptr_provenance operand to load/store.
Dec 21 2022, 1:41 PM · Restricted Project, Restricted Project
RalfJung added inline comments to D104268: [ptr_provenance] Introduce optional ptr_provenance operand to load/store.
Dec 21 2022, 9:41 AM · Restricted Project, Restricted Project
RalfJung added a comment to D69542: Full Restrict Support - single patch.

Hm I cannot find the diff that the 2nd and 3rd comment refer to in https://reviews.llvm.org/D104268? Am I looking at the wrong thing? It refers to the new "Scoped NoAlias Related Intrinsics" section of the LangRef.
Sorry for the noise. This patchset stuff is really confusing, I have no clue how to navigate them.

Dec 21 2022, 9:41 AM · Restricted Project, Restricted Project
RalfJung added a comment to D69542: Full Restrict Support - single patch.

Okay I will try to post the same comments there.

Dec 21 2022, 9:38 AM · Restricted Project, Restricted Project
RalfJung added a comment to D139785: [InstCombine] preserve signbit semantics of NAN with fold to fabs.

How was X initialized/observed with -NaN? Testing for bit-equality requires casting to integer. There's no way to do that comparison with FP values?

Dec 21 2022, 9:36 AM · Restricted Project, Restricted Project

Dec 11 2022

RalfJung added a comment to D139785: [InstCombine] preserve signbit semantics of NAN with fold to fabs.

Note that semantics of fneg/fabs is irrelevant for this optimization to be a problem. That's why I keep using float Y = true ? X1 : X2; as the example. To my knowledge nobody disputes that NaN < 0.0 must deterministically return false, so X < 0.0 ? -X : X will behave the same as false ? -X : X when X is a NAN, which is the same as just X -- I hope this much is uncontroversial. The only potential question then is whether float Y = X must preserve the NAN sign bit of X.

Dec 11 2022, 11:13 AM · Restricted Project, Restricted Project
RalfJung added a comment to D139785: [InstCombine] preserve signbit semantics of NAN with fold to fabs.

the signbit of NAN is never meaningful in real code,

Dec 11 2022, 10:09 AM · Restricted Project, Restricted Project
RalfJung added a comment to D139785: [InstCombine] preserve signbit semantics of NAN with fold to fabs.

Comparisons don't look at the sign of NaN. They are not bitwise operations like fabs/fneg.

Dec 11 2022, 9:23 AM · Restricted Project, Restricted Project
RalfJung added a comment to D139785: [InstCombine] preserve signbit semantics of NAN with fold to fabs.

I am a bit confused by the explanation. The signbit of NaN is "insignificant" for *some* operations like + in the sense that they just pick an arbitrary sign bit if the result is NaN (and so the sign bit of a NaN input has no effect). But it affects the behavior of >, fneg, and fabs (the operations involved here) in totally deterministic ways, right? When only >, fneg, fabs are used (and copies of the floating point value from one register to another), the sign bit is fully significant, like all the others.

Dec 11 2022, 8:52 AM · Restricted Project, Restricted Project

Dec 9 2022

RalfJung added inline comments to D69542: Full Restrict Support - single patch.
Dec 9 2022, 7:25 AM · Restricted Project, Restricted Project

Mar 7 2021

RalfJung added a comment to D98112: [LangRef] mention that the lifetime intrinsics' description in LangRef isn't everything.

LGTM, thanks. :)

Mar 7 2021, 10:19 AM · Restricted Project

Mar 6 2021

RalfJung added inline comments to D98112: [LangRef] mention that the lifetime intrinsics' description in LangRef isn't everything.
Mar 6 2021, 4:31 AM · Restricted Project
RalfJung added inline comments to D98112: [LangRef] mention that the lifetime intrinsics' description in LangRef isn't everything.
Mar 6 2021, 2:43 AM · Restricted Project

Mar 5 2021

RalfJung added a comment to D94002: [LangRef] Make lifetime intrinsic's semantics consistent with StackColoring's comment.

The exact semantics of lifetime.start depends on the pattern matching patterns in the stack coloring algorithm. So this intrinsic cannot be abused. It must be used for the uses it was created for only.

Mar 5 2021, 4:27 AM · Restricted Project
RalfJung added inline comments to D94002: [LangRef] Make lifetime intrinsic's semantics consistent with StackColoring's comment.
Mar 5 2021, 2:57 AM · Restricted Project

Jan 11 2021

RalfJung added inline comments to D94002: [LangRef] Make lifetime intrinsic's semantics consistent with StackColoring's comment.
Jan 11 2021, 4:12 AM · Restricted Project
RalfJung added a comment to D93376: [LangRef] Clarify the semantics of lifetime intrinsics.

You mean not supported by codegen, right? It would still have been possible for somebody to use them in some intermediate-only use of LLVM. I agree with your more important points, though.

Jan 11 2021, 4:01 AM · Restricted Project
RalfJung added a comment to D93820: [InstSimplify] Don't fold gep p, -p to null.

+ The base pointer has an in bounds address of the allocated object it is based on [...]

Jan 11 2021, 3:00 AM · Restricted Project

Dec 26 2020

RalfJung added a comment to D93376: [LangRef] Clarify the semantics of lifetime intrinsics.

I'd say that as a matter of policy the LangRef must be a document that people can rely on, else what's the point of having it? Since it is written as-if lifetime intrinsics could be applied rather freely

Dec 26 2020, 4:19 AM · Restricted Project

Dec 21 2020

RalfJung added a comment to D93376: [LangRef] Clarify the semantics of lifetime intrinsics.

Given the proposed semantic of lifetime.start, the program had UB semantics prior to inlining, we might not have known it statically but that is not an issue.

Dec 21 2020, 4:54 AM · Restricted Project

Dec 18 2020

RalfJung added a comment to D93376: [LangRef] Clarify the semantics of lifetime intrinsics.

This mixes semantic and implementation of an alloca. Let's not do that.

Dec 18 2020, 7:41 AM · Restricted Project
RalfJung added a comment to D93376: [LangRef] Clarify the semantics of lifetime intrinsics.

Two allocas with disjoint lifetimes should be able to be overlapped.

Dec 18 2020, 2:05 AM · Restricted Project
RalfJung added a comment to D93376: [LangRef] Clarify the semantics of lifetime intrinsics.

Why wouldn't/shouldn't it mean exactly what it means for stack allocations?
"The memory region specified in the lifetime start is dead until a lifetime start makes that memory accessible, ..."

Dec 18 2020, 12:55 AM · Restricted Project

Sep 4 2020

RalfJung added a comment to D86233: [LangRef] Define mustprogress attribute.

Shouldn't it be "maynotprogress" (or "maybenoprogress" or so)? *Every* function *may* progress, but only those with this marker are also allowed to not "make progress".

The way to read it is:
Every function *mustmakeprogress* unless it has the *mayprogress* attribute.

So "may" is either they do or they don't, both are fine.
And, "must" means they have to or UB.

Sep 4 2020, 8:58 AM · Restricted Project
RalfJung added a comment to D86233: [LangRef] Define mustprogress attribute.

Shouldn't it be "maynotprogress" (or "maybenoprogress" or so)? *Every* function *may* progress, but only those with this marker are also allowed to not "make progress".

Sep 4 2020, 2:03 AM · Restricted Project
RalfJung added a comment to D85393: [IR] Adds mustprogress as a LLVM IR attribute.

Changed name from noprogress to mayprogress for clarity.

Sep 4 2020, 2:02 AM · Restricted Project

Aug 28 2020

RalfJung added a comment to D85393: [IR] Adds mustprogress as a LLVM IR attribute.

I don't look at it as added or removed, for me it is a boolean flag, with a default that requires progress. It is more like nobuiltin, noduplicate, nomerge, returns_twice, .... The difference is that I don't see this being deduced. It is just a description of the input semantics, not a property we derive from the code (to describe the code). Unclear if the view makes a difference, let me know what u think :)

Aug 28 2020, 12:14 AM · Restricted Project

Aug 27 2020

RalfJung added a comment to D85393: [IR] Adds mustprogress as a LLVM IR attribute.

It depends on how you look at it:
From the perspective of optimizations we can do, sure absence is the one that enables more.
From the perspective of source behavior, it just distinguishes between two alternatives, so both option w/ and w/o flag have the same information value.

Aug 27 2020, 12:58 AM · Restricted Project

Aug 20 2020

RalfJung added a comment to D86233: [LangRef] Define mustprogress attribute.

This attribute indicates that the function is guaranteed to not make progress

Aug 20 2020, 1:09 AM · Restricted Project

Aug 10 2020

RalfJung added a comment to D85393: [IR] Adds mustprogress as a LLVM IR attribute.

It isn't contradiction, but about whether noprogress brings useful information or not, though.

Aug 10 2020, 2:01 AM · Restricted Project

Aug 9 2020

RalfJung added a comment to D65718: [LangRef] Document forward-progress requirement.

No happens-before rule is needed for sequential programs. The compiled program must exhibit all observable events in the same order as the source program.

Aug 9 2020, 7:11 AM · Restricted Project
RalfJung added a comment to D65718: [LangRef] Document forward-progress requirement.

Yes, C++ makes infinite loops UB (and C, under some conditions). This PR is about making LLVM support languages like JavaScript or Rust that do not have such UB.

Aug 9 2020, 3:48 AM · Restricted Project
RalfJung added a comment to D65718: [LangRef] Document forward-progress requirement.

I don't know what this statement is based on.

Aug 9 2020, 3:31 AM · Restricted Project

Aug 8 2020

RalfJung added a comment to D65718: [LangRef] Document forward-progress requirement.

If there are no accesses to the variable, there are no calls of the function. Throwing away the loop is valid, analysis of whether it is finite is not required.

Aug 8 2020, 1:31 AM · Restricted Project

Aug 6 2020

RalfJung added a comment to D85393: [IR] Adds mustprogress as a LLVM IR attribute.

Either should do the trick after a while given that all optimizations need to preserve/handle attribute already, you cannot just drop them and so noprogress should be sufficient.

Aug 6 2020, 9:06 AM · Restricted Project
RalfJung added a comment to D85393: [IR] Adds mustprogress as a LLVM IR attribute.

I am going solely off the description here as I am not familiar with LLVM internals. Looking forward to the LangRef update. :)

Aug 6 2020, 1:47 AM · Restricted Project

Nov 27 2019

RalfJung added inline comments to D61652: [Attr] Introduce dereferenceable_globally.
Nov 27 2019, 9:59 AM · Restricted Project

Nov 21 2019

RalfJung added inline comments to D61652: [Attr] Introduce dereferenceable_globally.
Nov 21 2019, 12:56 AM · Restricted Project

Aug 15 2019

RalfJung added inline comments to D65718: [LangRef] Document forward-progress requirement.
Aug 15 2019, 11:18 PM · Restricted Project
RalfJung added a comment to D65718: [LangRef] Document forward-progress requirement.

A fence is not an atomic operation on its own in C++. See http://eel.is/c++draft/atomics.fences where it says "is a synchronization operation *if* [...]".

Aug 15 2019, 11:15 PM · Restricted Project

Aug 12 2019

RalfJung added inline comments to D65718: [LangRef] Document forward-progress requirement.
Aug 12 2019, 2:16 AM · Restricted Project
RalfJung added a comment to D65718: [LangRef] Document forward-progress requirement.

Does this need a side effect? Per the current definition, no. Per C++, yes.

Aug 12 2019, 2:13 AM · Restricted Project

Jul 23 2019

RalfJung added a comment to D65134: Clarify where the indirect UB due to write-write races comes from.

I cannot land commits myself, could you help with that?

Jul 23 2019, 9:25 AM · Restricted Project
RalfJung created D65134: Clarify where the indirect UB due to write-write races comes from.
Jul 23 2019, 2:16 AM · Restricted Project

Feb 26 2019

RalfJung added a comment to D57600: update docs of memcpy/memmove/memset re: alignment and len=0.

I fixed the typo.

Feb 26 2019, 3:28 AM · Restricted Project
RalfJung updated the diff for D57600: update docs of memcpy/memmove/memset re: alignment and len=0.

Fix typo.

Feb 26 2019, 3:28 AM · Restricted Project
RalfJung added a comment to D57600: update docs of memcpy/memmove/memset re: alignment and len=0.

Please make sure you have llvm-commits added as a subscriber when creating patches in the future

Feb 26 2019, 1:45 AM · Restricted Project

Feb 1 2019

RalfJung created D57600: update docs of memcpy/memmove/memset re: alignment and len=0.
Feb 1 2019, 10:08 AM · Restricted Project