[PATCH] D48134: [CodeGen] make nan builtins pure rather than const (PR37778)
spatel added a comment. In https://reviews.llvm.org/D48134#1131626, @rsmith wrote: > Can we mark these as `argmemonly`? I wasn't aware of that one, but it sounds accurate for nan() and friends: argmemonly This attribute indicates that the only memory accesses inside function are loads and stores from objects pointed to by its pointer-typed arguments, with arbitrary offsets. Or in other words, all memory operations in the function can refer to memory only using pointers based on its function arguments. Note that argmemonly can be used together with readonly attribute in order to specify that function reads only from its arguments. But as Roman noted, we're going to have to update the list of possibilities to include "NoAliasAttr"? And might need to adjust the logic where that gets mapped to the LLVM attr. Repository: rL LLVM https://reviews.llvm.org/D48134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48134: [CodeGen] make nan builtins pure rather than const (PR37778)
lebedev.ri added a comment. In https://reviews.llvm.org/D48134#1131626, @rsmith wrote: > Can we mark these as `argmemonly`? Header comment in `include/clang/Basic/Builtins.def` does not list that as a possibility. Repository: rL LLVM https://reviews.llvm.org/D48134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48134: [CodeGen] make nan builtins pure rather than const (PR37778)
rsmith added a comment. Can we mark these as `argmemonly`? Repository: rL LLVM https://reviews.llvm.org/D48134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48134: [CodeGen] make nan builtins pure rather than const (PR37778)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334628: [CodeGen] make nan builtins pure rather than const
(PR37778) (authored by spatel, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D48134?vs=151187&id=151202#toc
Repository:
rL LLVM
https://reviews.llvm.org/D48134
Files:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/test/CodeGen/math-builtins.c
Index: cfe/trunk/include/clang/Basic/Builtins.def
===
--- cfe/trunk/include/clang/Basic/Builtins.def
+++ cfe/trunk/include/clang/Basic/Builtins.def
@@ -137,14 +137,14 @@
BUILTIN(__builtin_modf , "ddd*" , "Fn")
BUILTIN(__builtin_modff, "fff*" , "Fn")
BUILTIN(__builtin_modfl, "LdLdLd*", "Fn")
-BUILTIN(__builtin_nan, "dcC*" , "ncF")
-BUILTIN(__builtin_nanf, "fcC*" , "ncF")
-BUILTIN(__builtin_nanl, "LdcC*", "ncF")
-BUILTIN(__builtin_nanf128, "LLdcC*", "ncF")
-BUILTIN(__builtin_nans, "dcC*" , "ncF")
-BUILTIN(__builtin_nansf, "fcC*" , "ncF")
-BUILTIN(__builtin_nansl, "LdcC*", "ncF")
-BUILTIN(__builtin_nansf128, "LLdcC*", "ncF")
+BUILTIN(__builtin_nan, "dcC*" , "FnU")
+BUILTIN(__builtin_nanf, "fcC*" , "FnU")
+BUILTIN(__builtin_nanl, "LdcC*", "FnU")
+BUILTIN(__builtin_nanf128, "LLdcC*", "FnU")
+BUILTIN(__builtin_nans, "dcC*" , "FnU")
+BUILTIN(__builtin_nansf, "fcC*" , "FnU")
+BUILTIN(__builtin_nansl, "LdcC*", "FnU")
+BUILTIN(__builtin_nansf128, "LLdcC*", "FnU")
BUILTIN(__builtin_powi , "ddi" , "Fnc")
BUILTIN(__builtin_powif, "ffi" , "Fnc")
BUILTIN(__builtin_powil, "LdLdi", "Fnc")
Index: cfe/trunk/test/CodeGen/math-builtins.c
===
--- cfe/trunk/test/CodeGen/math-builtins.c
+++ cfe/trunk/test/CodeGen/math-builtins.c
@@ -90,25 +90,25 @@
__builtin_nan(c);__builtin_nanf(c); __builtin_nanl(c);
__builtin_nanf128(c);
-// NO__ERRNO: declare double @nan(i8*) [[READNONE]]
-// NO__ERRNO: declare float @nanf(i8*) [[READNONE]]
-// NO__ERRNO: declare x86_fp80 @nanl(i8*) [[READNONE]]
-// NO__ERRNO: declare fp128 @nanf128(i8*) [[READNONE]]
-// HAS_ERRNO: declare double @nan(i8*) [[READNONE:#[0-9]+]]
-// HAS_ERRNO: declare float @nanf(i8*) [[READNONE]]
-// HAS_ERRNO: declare x86_fp80 @nanl(i8*) [[READNONE]]
-// HAS_ERRNO: declare fp128 @nanf128(i8*) [[READNONE]]
+// NO__ERRNO: declare double @nan(i8*) [[PURE:#[0-9]+]]
+// NO__ERRNO: declare float @nanf(i8*) [[PURE]]
+// NO__ERRNO: declare x86_fp80 @nanl(i8*) [[PURE]]
+// NO__ERRNO: declare fp128 @nanf128(i8*) [[PURE]]
+// HAS_ERRNO: declare double @nan(i8*) [[PURE:#[0-9]+]]
+// HAS_ERRNO: declare float @nanf(i8*) [[PURE]]
+// HAS_ERRNO: declare x86_fp80 @nanl(i8*) [[PURE]]
+// HAS_ERRNO: declare fp128 @nanf128(i8*) [[PURE]]
__builtin_nans(c);__builtin_nansf(c); __builtin_nansl(c);
__builtin_nansf128(c);
-// NO__ERRNO: declare double @nans(i8*) [[READNONE]]
-// NO__ERRNO: declare float @nansf(i8*) [[READNONE]]
-// NO__ERRNO: declare x86_fp80 @nansl(i8*) [[READNONE]]
-// NO__ERRNO: declare fp128 @nansf128(i8*) [[READNONE]]
-// HAS_ERRNO: declare double @nans(i8*) [[READNONE]]
-// HAS_ERRNO: declare float @nansf(i8*) [[READNONE]]
-// HAS_ERRNO: declare x86_fp80 @nansl(i8*) [[READNONE]]
-// HAS_ERRNO: declare fp128 @nansf128(i8*) [[READNONE]]
+// NO__ERRNO: declare double @nans(i8*) [[PURE]]
+// NO__ERRNO: declare float @nansf(i8*) [[PURE]]
+// NO__ERRNO: declare x86_fp80 @nansl(i8*) [[PURE]]
+// NO__ERRNO: declare fp128 @nansf128(i8*) [[PURE]]
+// HAS_ERRNO: declare double @nans(i8*) [[PURE]]
+// HAS_ERRNO: declare float @nansf(i8*) [[PURE]]
+// HAS_ERRNO: declare x86_fp80 @nansl(i8*) [[PURE]]
+// HAS_ERRNO: declare fp128 @nansf128(i8*) [[PURE]]
__builtin_pow(f,f);__builtin_powf(f,f); __builtin_powl(f,f);
@@ -188,7 +188,7 @@
// NO__ERRNO: declare double @cbrt(double) [[READNONE]]
// NO__ERRNO: declare float @cbrtf(float) [[READNONE]]
// NO__ERRNO: declare x86_fp80 @cbrtl(x86_fp80) [[READNONE]]
-// HAS_ERRNO: declare double @cbrt(double) [[READNONE]]
+// HAS_ERRNO: declare double @cbrt(double) [[READNONE:#[0-9]+]]
// HAS_ERRNO: declare float @cbrtf(float) [[READNONE]]
// HAS_ERRNO: declare x86_fp80 @cbrtl(x86_fp80) [[READNONE]]
@@ -581,9 +581,11 @@
// NO__ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
// NO__ERRNO: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
// NO__ERRNO: attributes [[NOT_READNONE]] = { nounwind "correctly{{.*}} }
+// NO__ERRNO: attributes [[PURE]] = { {{.*}}readonly{{.*}} }
// HAS_ERRNO: attributes [[NOT_READNONE]] = { nounwind "correctly{{.*}} }
// HAS_ERRNO: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
+// HAS_ERRNO: attributes [[PURE]] = { {{.*}}readonly{{.*}} }
// HAS_ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
// HAS_ERRNO_GNU: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
Index: cfe/trunk/include/clan
[PATCH] D48134: [CodeGen] make nan builtins pure rather than const (PR37778)
lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Makes sense. https://reviews.llvm.org/D48134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48134: [CodeGen] make nan builtins pure rather than const (PR37778)
spatel created this revision.
spatel added reviewers: gfalcon, lebedev.ri.
Herald added a subscriber: mcrosier.
https://bugs.llvm.org/show_bug.cgi?id=37778
...shows a miscompile resulting from marking nan builtins as 'const'.
The nan libcalls/builtins take a pointer argument:
http://www.cplusplus.com/reference/cmath/nan-function/
...and the chars dereferenced by that arg are used to fill in the NaN constant
payload bits.
"const" means that the pointer argument isn't dereferenced. That's translated
to "readnone" in LLVM.
"pure" means that the pointer argument may be dereferenced. That's translated
to "readonly" in LLVM.
This change prevents the IR optimizer from killing the lead-up to the nan call
here:
double a() {
char buf[4];
buf[0] = buf[1] = buf[2] = '9';
buf[3] = '\0';
return __builtin_nan(buf);
}
...the optimizer isn't currently able to simplify this to a constant as we
might hope, but this patch should solve the miscompile.
https://reviews.llvm.org/D48134
Files:
include/clang/Basic/Builtins.def
test/CodeGen/math-builtins.c
Index: test/CodeGen/math-builtins.c
===
--- test/CodeGen/math-builtins.c
+++ test/CodeGen/math-builtins.c
@@ -90,25 +90,25 @@
__builtin_nan(c);__builtin_nanf(c); __builtin_nanl(c);
__builtin_nanf128(c);
-// NO__ERRNO: declare double @nan(i8*) [[READNONE]]
-// NO__ERRNO: declare float @nanf(i8*) [[READNONE]]
-// NO__ERRNO: declare x86_fp80 @nanl(i8*) [[READNONE]]
-// NO__ERRNO: declare fp128 @nanf128(i8*) [[READNONE]]
-// HAS_ERRNO: declare double @nan(i8*) [[READNONE:#[0-9]+]]
-// HAS_ERRNO: declare float @nanf(i8*) [[READNONE]]
-// HAS_ERRNO: declare x86_fp80 @nanl(i8*) [[READNONE]]
-// HAS_ERRNO: declare fp128 @nanf128(i8*) [[READNONE]]
+// NO__ERRNO: declare double @nan(i8*) [[PURE:#[0-9]+]]
+// NO__ERRNO: declare float @nanf(i8*) [[PURE]]
+// NO__ERRNO: declare x86_fp80 @nanl(i8*) [[PURE]]
+// NO__ERRNO: declare fp128 @nanf128(i8*) [[PURE]]
+// HAS_ERRNO: declare double @nan(i8*) [[PURE:#[0-9]+]]
+// HAS_ERRNO: declare float @nanf(i8*) [[PURE]]
+// HAS_ERRNO: declare x86_fp80 @nanl(i8*) [[PURE]]
+// HAS_ERRNO: declare fp128 @nanf128(i8*) [[PURE]]
__builtin_nans(c);__builtin_nansf(c); __builtin_nansl(c);
__builtin_nansf128(c);
-// NO__ERRNO: declare double @nans(i8*) [[READNONE]]
-// NO__ERRNO: declare float @nansf(i8*) [[READNONE]]
-// NO__ERRNO: declare x86_fp80 @nansl(i8*) [[READNONE]]
-// NO__ERRNO: declare fp128 @nansf128(i8*) [[READNONE]]
-// HAS_ERRNO: declare double @nans(i8*) [[READNONE]]
-// HAS_ERRNO: declare float @nansf(i8*) [[READNONE]]
-// HAS_ERRNO: declare x86_fp80 @nansl(i8*) [[READNONE]]
-// HAS_ERRNO: declare fp128 @nansf128(i8*) [[READNONE]]
+// NO__ERRNO: declare double @nans(i8*) [[PURE]]
+// NO__ERRNO: declare float @nansf(i8*) [[PURE]]
+// NO__ERRNO: declare x86_fp80 @nansl(i8*) [[PURE]]
+// NO__ERRNO: declare fp128 @nansf128(i8*) [[PURE]]
+// HAS_ERRNO: declare double @nans(i8*) [[PURE]]
+// HAS_ERRNO: declare float @nansf(i8*) [[PURE]]
+// HAS_ERRNO: declare x86_fp80 @nansl(i8*) [[PURE]]
+// HAS_ERRNO: declare fp128 @nansf128(i8*) [[PURE]]
__builtin_pow(f,f);__builtin_powf(f,f); __builtin_powl(f,f);
@@ -188,7 +188,7 @@
// NO__ERRNO: declare double @cbrt(double) [[READNONE]]
// NO__ERRNO: declare float @cbrtf(float) [[READNONE]]
// NO__ERRNO: declare x86_fp80 @cbrtl(x86_fp80) [[READNONE]]
-// HAS_ERRNO: declare double @cbrt(double) [[READNONE]]
+// HAS_ERRNO: declare double @cbrt(double) [[READNONE:#[0-9]+]]
// HAS_ERRNO: declare float @cbrtf(float) [[READNONE]]
// HAS_ERRNO: declare x86_fp80 @cbrtl(x86_fp80) [[READNONE]]
@@ -581,9 +581,11 @@
// NO__ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
// NO__ERRNO: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
// NO__ERRNO: attributes [[NOT_READNONE]] = { nounwind "correctly{{.*}} }
+// NO__ERRNO: attributes [[PURE]] = { {{.*}}readonly{{.*}} }
// HAS_ERRNO: attributes [[NOT_READNONE]] = { nounwind "correctly{{.*}} }
// HAS_ERRNO: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
+// HAS_ERRNO: attributes [[PURE]] = { {{.*}}readonly{{.*}} }
// HAS_ERRNO: attributes [[READNONE]] = { {{.*}}readnone{{.*}} }
// HAS_ERRNO_GNU: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -137,14 +137,14 @@
BUILTIN(__builtin_modf , "ddd*" , "Fn")
BUILTIN(__builtin_modff, "fff*" , "Fn")
BUILTIN(__builtin_modfl, "LdLdLd*", "Fn")
-BUILTIN(__builtin_nan, "dcC*" , "ncF")
-BUILTIN(__builtin_nanf, "fcC*" , "ncF")
-BUILTIN(__builtin_nanl, "LdcC*", "ncF")
-BUILTIN(__builtin_nanf128, "LLdcC*", "ncF")
-BUILTIN(__builtin_nans, "dcC*" , "ncF")
-BUILTIN(__builtin_nansf, "fcC*" , "ncF")
-BUILTIN(__builtin_nansl, "LdcC*", "
