On Mon, Dec 7, 2015 at 9:51 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
> On Mon, Dec 7, 2015 at 5:26 PM, Reid Kleckner via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> It wasn't Chrome, it was some internal dependency on torch-cephes. I >> submitted a patch for it upstream: >> >> https://github.com/deepmind/torch-cephes/commit/9c4a97c90dc200ecbecb883e7230fe3c847954df >> It's not a pretty, though. >> >> I know LLVM IR rules are not C++ rules, but LLVM generally believes in >> NaN. It will speculate things like fdiv above branches, leading to traps >> when people configure the FPU appropriately and then conditionally divide >> by zero. It would be more consistent for Clang to follow the LLVM direction >> here. The fact that C++ says that FP div-by-zero is UB might just be a bug >> in the standard, or a grey area that couldn't be filled in. >> > > Right, I'm not suggesting we treat floating-point division by zero as > "true UB". But the fact we have a sanitizer for it means that our behavior > is not, and cannot be, "just give a NaN or Inf". It's a choice of either > "give me a NaN or Inf" or "give me a sanitizer diagnostic, followed by a > NaN or Inf". And that means it's not constant in the general case, because > evaluating it has runtime side-effects. > The mere existence of a sanitizer shouldn't be what determines our behavior in the general case. We also have a sanitizer for unsigned overflow... > > Prior to this patch, Clang would determine that such divisions had no > side-effects, and speculate them *and the corresponding sanitizer checks*, > resulting in sanitizer false positives on correct code. > > >> On Mon, Dec 7, 2015 at 2:37 PM, David Majnemer <david.majne...@gmail.com> >> wrote: >> >>> >>> >>> On Mon, Dec 7, 2015 at 5:25 PM, Hans Wennborg <h...@chromium.org> wrote: >>> >>>> On Mon, Dec 7, 2015 at 2:14 PM, David Majnemer < >>>> david.majne...@gmail.com> wrote: >>>> > >>>> > >>>> > On Mon, Dec 7, 2015 at 4:32 PM, Richard Smith via cfe-commits >>>> > <cfe-commits@lists.llvm.org> wrote: >>>> >> >>>> >> On Mon, Dec 7, 2015 at 7:25 AM, Joerg Sonnenberger via cfe-commits >>>> >> <cfe-commits@lists.llvm.org> wrote: >>>> >>> >>>> >>> On Thu, Dec 03, 2015 at 01:36:22AM -0000, Richard Smith via >>>> cfe-commits >>>> >>> wrote: >>>> >>> > Author: rsmith >>>> >>> > Date: Wed Dec 2 19:36:22 2015 >>>> >>> > New Revision: 254574 >>>> >>> > >>>> >>> > URL: http://llvm.org/viewvc/llvm-project?rev=254574&view=rev >>>> >>> > Log: >>>> >>> > PR17381: Treat undefined behavior during expression evaluation as >>>> an >>>> >>> > unmodeled >>>> >>> > side-effect, so that we don't allow speculative evaluation of such >>>> >>> > expressions >>>> >>> > during code generation. >>>> >>> > >>>> >>> > This caused a diagnostic quality regression, so fix constant >>>> expression >>>> >>> > diagnostics to prefer either the first "can't be constant folded" >>>> >>> > diagnostic or >>>> >>> > the first "not a constant expression" diagnostic depending on the >>>> kind >>>> >>> > of >>>> >>> > evaluation we're doing. This was always the intent, but didn't >>>> quite >>>> >>> > work >>>> >>> > correctly before. >>>> >>> > >>>> >>> > This results in certain initializers that used to be constant >>>> >>> > initializers to >>>> >>> > no longer be; in particular, things like: >>>> >>> > >>>> >>> > float f = 1e100; >>>> >>> > >>>> >>> > are no longer accepted in C. This seems appropriate, as such >>>> constructs >>>> >>> > would >>>> >>> > lead to code being executed if sanitizers are enabled. >>>> >>> >>>> >>> This leads to some pretty annoying regressions as it now seems to be >>>> >>> impossible to use NaN or infinites as constant initializers. >>>> Expressions >>>> >>> like 0.0 / 0.0, 1.0 / 0.0 and -1.0 / 0.0 are perfectly well defined >>>> >>> under normal IEEE rules, so they shouldn't be rejected. >>>> >> >>>> >> >>>> >> Well, we have a problem. The evaluation semantics of these >>>> expressions >>>> >> requires code to execute in some build modes (in particular, with >>>> sanitizers >>>> >> enabled), and thus has a side-effect. >>>> >> >>>> >> I'm inclined to relax the restriction added in this change for the >>>> >> specific case of global variables in C, since (as you say) there is >>>> a fair >>>> >> amount of code using divide-by-zero as a "portable" way of >>>> generating an inf >>>> >> or nan. >>>> >> >>>> >>> Worse, it seems >>>> >>> even using __builtin_nan() for example doesn't work. >>>> >> >>>> >> >>>> >> __builtin_nan() works fine for me, can you provide a testcase? >>>> >> >>>> >>> I'm not even sure about the example given in the commit message, how >>>> >>> exactly is that undefined behavior? >>>> >> >>>> >> >>>> >> C11 6.3.1.5/1: "If the value being converted is outside the range of >>>> >> values that can be represented, the behavior is undefined." >>>> > >>>> > >>>> > I don't think we want to make the UB here true UB. It would mean >>>> that code >>>> > which expected to get NaN might get undef, even outside of constant >>>> > expression evaluation. The implementation defined behavior of >>>> providing NaN >>>> > seems more friendly... IIRC, this broke Chrome recently because >>>> folks were >>>> > doing this in C++. Hans, do you remember the details? >>>> >>>> Hmm, it doesn't ring a bell, but my memory sometimes fails me. Didn't >>>> you and Reid look at something like this the other day? (But maybe >>>> that was in internal code?) >>>> >>> >>> Ah, I think you are right... Reid, do you happen to recall? >>> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
