[CodeGen] make nan builtins pure rather than const (PR37778)

[spatel]

Summary

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.

[lebedev.ri]

Makes sense.

[rsmith]

Can we mark these as argmemonly?

[lebedev.ri]

In 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.

[spatel]

In 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.

cfe/trunk/include/clang/Basic/Builtins.def

 BUILTIN(__builtin_labs , "LiLi"  , "Fnc")
 BUILTIN(__builtin_llabs, "LLiLLi", "Fnc")
 BUILTIN(__builtin_ldexp , "ddi"  , "Fne")
 BUILTIN(__builtin_ldexpf, "ffi"  , "Fne")
 BUILTIN(__builtin_ldexpl, "LdLdi", "Fne")
 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")
 BUILTIN(__builtin_pow , "ddd"  , "Fne")
 BUILTIN(__builtin_powf, "fff"  , "Fne")
 BUILTIN(__builtin_powl, "LdLdLd", "Fne")
 
 // Standard unary libc/libm functions with double/float/long double variants:

cfe/trunk/test/CodeGen/math-builtins.c

 // NO__ERRNO: declare float @modff(float, float*) [[NOT_READNONE]]
 // NO__ERRNO: declare x86_fp80 @modfl(x86_fp80, x86_fp80*) [[NOT_READNONE]]
 // HAS_ERRNO: declare double @modf(double, double*) [[NOT_READNONE]]
 // HAS_ERRNO: declare float @modff(float, float*) [[NOT_READNONE]]
 // HAS_ERRNO: declare x86_fp80 @modfl(x86_fp80, x86_fp80*) [[NOT_READNONE]]
 
   __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);
 
 // NO__ERRNO: declare double @llvm.pow.f64(double, double) [[READNONE_INTRINSIC]]
 // NO__ERRNO: declare float @llvm.pow.f32(float, float) [[READNONE_INTRINSIC]]
 // NO__ERRNO: declare x86_fp80 @llvm.pow.f80(x86_fp80, x86_fp80) [[READNONE_INTRINSIC]]
 // HAS_ERRNO: declare double @pow(double, double) [[NOT_READNONE]]
 // HAS_ERRNO: declare float @powf(float, float) [[NOT_READNONE]]

 // HAS_ERRNO: declare float @atanhf(float) [[NOT_READNONE]]
 // HAS_ERRNO: declare x86_fp80 @atanhl(x86_fp80) [[NOT_READNONE]]
 
   __builtin_cbrt(f);       __builtin_cbrtf(f);      __builtin_cbrtl(f);
 
 // 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]]
 
   __builtin_ceil(f);       __builtin_ceilf(f);      __builtin_ceill(f);
 
 // NO__ERRNO: declare double @llvm.ceil.f64(double) [[READNONE_INTRINSIC]]
 // NO__ERRNO: declare float @llvm.ceil.f32(float) [[READNONE_INTRINSIC]]
 // NO__ERRNO: declare x86_fp80 @llvm.ceil.f80(x86_fp80) [[READNONE_INTRINSIC]]

 // HAS_ERRNO: declare float @llvm.trunc.f32(float) [[READNONE_INTRINSIC]]
 // HAS_ERRNO: declare x86_fp80 @llvm.trunc.f80(x86_fp80) [[READNONE_INTRINSIC]]
 };
 
 
 // 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{{.*}} }
 // HAS_ERRNO_ANDROID: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }
 // HAS_ERRNO_WIN: attributes [[READNONE_INTRINSIC]] = { {{.*}}readnone{{.*}} }