andrew.w.kaylor added inline comments.
================ Comment at: clang/include/clang/Basic/FPOptions.def:30 OPTION(BFloat16ExcessPrecision, LangOptions::ExcessPrecisionKind, 2, FPEvalMethod) +OPTION(MathErrno, bool, 1, BFloat16ExcessPrecision) #undef OPTION ---------------- Does this mean MathErrno is tracked in both LangOpts and FPOptions? ================ Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2248 FD->hasAttr<AsmLabelAttr>() ? 0 : BuiltinID; + bool MathErrnoOverrride = false; + if (E->hasStoredFPFeatures()) { ---------------- You should add a comment here explaining what this is doing. I don't think it's obvious apart from the description of this patch. ================ Comment at: clang/lib/CodeGen/CGCall.cpp:2384 + if (HasOptnone && !getLangOpts().MathErrno) + OptNone = false; ---------------- I don't understand what this is doing. ================ Comment at: clang/lib/CodeGen/CodeGenModule.h:611 void clear(); + bool OptNone = true; ---------------- Why is this a module level flag, and why does it default to true? ================ Comment at: clang/test/CodeGen/math-errno.c:2 +// Tests that at -O2 math-errno is taken into account. Same than MSVC. +// RUN: %clang_cc1 -Wno-implicit-function-declaration -fmath-errno \ +// RUN: -O2 -emit-llvm -o - %s \ ---------------- Isn't math-errno true by default when fast-math isn't used? ================ Comment at: clang/test/CodeGen/math-errno.c:33 + +__attribute__((optnone)) +float f4(float x) { ---------------- Can you add a runline with -O0. That should prevent all instances of the intrinsics, right? ================ Comment at: clang/test/CodeGen/math-errno.c:49 +// FAST-LABEL: define dso_local nofpclass(nan inf) float @f4(float noundef nofpclass(nan inf) {{.*}}) +// FAST: call reassoc nnan ninf nsz arcp afn nofpclass(nan inf) float @sqrtf(float noundef nofpclass(nan inf) %0) #[[ATTR0:[0-9]+]] + ---------------- I think the 'afn' flag here is a problem. The backend has no concept of errno, so 'afn' will be treated as allowing the function to be replaced. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151834/new/ https://reviews.llvm.org/D151834 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits