Change implementation of round_f* in CodeGen_C to use nearbyint() to match CodeGen_LLVM

[vksnk]

round_f32/round_f64 implementations at

Halide/src/runtime/posix_math.ll

Line 130 in 408a277

define weak_odr float @round_f32(float %x) nounwind uwtable readnone alwaysinline {

use nearbyint.

[steven-johnson]

Wouldn't it be better to change LLVM to use round instead? (IIRC, nearbyint uses the current rounding mode)

[dsharletg]

We thought that the LLVM backend is much more heavily used, so it would make sense to not change that behavior?

[dsharletg]

That said: round does sound better than nearbyint (you are correct, round does not use the rounding mode, and nearbyint does not).

[steven-johnson]

We thought that the LLVM backend is much more heavily used, so it would make sense to not change that behavior?

Agreed, so I guess this is really "should we migrate the behavior to match" -- IMHO stuff that relies on current rounding mode is a flaky test waiting to happen. Maybe we should consider adding nearbyint and then having each match their C++ equivalent?

[vksnk]

Wouldn't it be better to change LLVM to use round instead? (IIRC, nearbyint uses the current rounding mode)

I guess it might be better in the sense that what round would be doing will be more aligned with expectations. On the other hand, LLVM backends are more widely used, so change in behavior there is riskier and in this case changing CodeGen_C to match is better.

In any case, the goal is to get matching behavior between two, so I'll be happy to change LLVM case instead if that's preferable.

[dsharletg]

I think we should submit this (harmonizing LLVM and C without messing with the likely more common case), and then file an issue for switching from nearbyint to round (regardless of target).

[steven-johnson]

I think we should submit this (harmonizing LLVM and C without messing with the likely more common case), and then file an issue for switching from nearbyint to round (regardless of target).

+1