RFR: 8273139: C2: assert(f <= 1 && f >= 0) failed: Incorrect frequency [v2]

Emanuel Peter duke at openjdk.java.net
Tue Jan 18 08:35:59 UTC 2022


> Relaxed assert which can be triggered falsely upon an unfortunate sequence of float additions, such that the sum of all probabilities exceeds 1 by some very small amount in the order of float precision.
> 
> Before: assert(f <= 1 && f >= 0)
> Now: assert(f >= 0), truncate f to be <=1
> 
> Frequency computation is done with floats, which are calculated through counting occurrences and dividing by totals.
> This is done in multiple locations in the code.
> 
> We considered three options (in conversation with @rwestrel @chhagedorn @tobiasholenstein ):
> 1. Consistently use `fesetround` to correctly round up/down (depends on context) if the results are ever used for frequency/probability calculations (many locations, multiple files). This option requires more code and maintainance. Implementing and testing is difficult. This is fragile if new code is added that impacts frequency computation - would have to remember to round correctly.
> 2. Modify assert to `f <= 1+eps && 1 >= 0`, where `eps` is related to float precision, and is as close to zero as possible. However, since there can be an arbitrary number of additions, this error could grow arbitrarily. It is thus not evident how to determine `eps`.
> 3. Drop the `f <= 1` condition (our choice). The exact comparison is inadequate for floats, and there is no evident replacement. This requires less code, is easier to maintain. Disadvantage: a developer may break something and not realize because the assert is no longer present. The impact would most likely be limited to performance, and not crash the VM or cause incorrect execution.
> 
> Checked that tests are not affected.

Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:

  collapse two lines of code

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7113/files
  - new: https://git.openjdk.java.net/jdk/pull/7113/files/a2a42290..b26bb66a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7113&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7113&range=00-01

  Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7113.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7113/head:pull/7113

PR: https://git.openjdk.java.net/jdk/pull/7113


More information about the hotspot-compiler-dev mailing list