[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-31 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In https://reviews.llvm.org/D39204#912084, @efriedma wrote: > Let's do this one step at a time; first this patch, then fix the __builtin_* > functions to use "e", then add all the missing cases > CodeGenFunction::EmitBuiltinExpr. Sounds good. > This patch LGTM, assumi

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-31 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Let's do this one step at a time; first this patch, then fix the __builtin_* functions to use "e", then add all the missing cases CodeGenFunction::EmitBuiltinExpr. This patch LGTM, assumi

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-31 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In https://reviews.llvm.org/D39204#911316, @efriedma wrote: > > I was going to include the builtins too, but that exposes another bug > > (marked here with FIXME) - the builtins are all defined 'const'. > > Probably just need to change c->e in Builtins.def? Yes - at lea

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-30 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > I was going to include the builtins too, but that exposes another bug (marked > here with FIXME) - the builtins are all defined 'const'. Probably just need to change c->e in Builtins.def? > This does make me curious about the use-case of libm-equivalent builtins. If

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-30 Thread Sanjay Patel via Phabricator via cfe-commits
spatel updated this revision to Diff 120902. spatel added a comment. Patch updated: As suggested, I've morphed this patch to just handle sqrt libcalls based on the updated LLVM intrinsic definition. I was going to include the builtins too, but that exposes another bug (marked here with FIXME) -

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-26 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment. In https://reviews.llvm.org/D39204#905860, @efriedma wrote: > I think you're understanding the semantics correctly. > > For r265521, look again at the implementation of > llvm::checkUnaryFloatSignature; if "I.onlyReadsMemory()" is true, we somehow > proved the call does

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-24 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I think you're understanding the semantics correctly. For r265521, look again at the implementation of llvm::checkUnaryFloatSignature; if "I.onlyReadsMemory()" is true, we somehow proved the call doesn't set errno (mostly likely because we know something about the tar

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-24 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. Working my way through the stack: does the sqrt LangRef change mean we can revert https://reviews.llvm.org/rL265521? What allows us to transform any of those libm calls that set errno to vectors in the first place? I'm even more confused than usual, but if I can find a w

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-24 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In https://reviews.llvm.org/D39204#904312, @efriedma wrote: > The gcc documentation says "GCC includes built-in versions of many of the > functions in the standard C library. These functions come in two forms: one > whose names start with the `__builtin_` prefix, and the

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-23 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. The gcc documentation says "GCC includes built-in versions of many of the functions in the standard C library. These functions come in two forms: one whose names start with the `__builtin_` prefix, and the other without. Both forms have the same type (including prototy

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-23 Thread Sanjay Patel via Phabricator via cfe-commits
spatel created this revision. Herald added a subscriber: mcrosier. This patch is intended to answer a question raised in PR27435: https://bugs.llvm.org/show_bug.cgi?id=27435 Is a programmer using __builtin_sqrt() invoking the compiler's intrinsic definition of sqrt or the mathlib definition of s