Re: [PATCH] D12793: Three new security overflow builtins with generic argument types

2015-09-25 Thread John McCall via cfe-commits
rjmccall added a comment. X and Y aren't unreasonable for the operands, although you could also use "left" and "right" (or LHS/RHS), especially since it's significant for subtraction. R is short for "result" and should be spelled out. E is presumably short for "encompassing", but that is not

Re: [PATCH] D12793: Three new security overflow builtins with generic argument types

2015-09-24 Thread David Grayson via cfe-commits
DavidEGrayson removed rL LLVM as the repository for this revision. DavidEGrayson updated this revision to Diff 35592. DavidEGrayson added a comment. I have changed the patch to incorporate most of John McCall's feedback. The only thing I didn't act on was the suggestion to change the names of

Re: [PATCH] D12793: Three new security overflow builtins with generic argument types

2015-09-23 Thread David Grayson via cfe-commits
DavidEGrayson added a comment. Hi John. I thought about your comment on line 1601 somewhat carefully and tried to imagine what it would look like. See my inline comment. Also, by the way, do you know if the results of the LLVM intrinsics like `llvm.smul.with.overflow.*` are guaranteed to be

Re: [PATCH] D12793: Three new security overflow builtins with generic argument types

2015-09-23 Thread John McCall via cfe-commits
rjmccall added a comment. Yes, the main result is defined to be the true value mod 2^k even when overflow occurs. We do use the intrinsics to implement the existing fixed-width GCC builtins, which are meant to be useful not just for security checking, but for implementing arbitrary-precision

Re: [PATCH] D12793: Three new security overflow builtins with generic argument types

2015-09-22 Thread David Grayson via cfe-commits
DavidEGrayson added a comment. Thanks for looking at this patch, John! I disagreed with your comment about calculating the encompassing type, but I will incorporate everything else and make a new patch tonight or within a few days. Comment at:

Re: [PATCH] D12793: Three new security overflow builtins with generic argument types

2015-09-21 Thread John McCall via cfe-commits
rjmccall added a comment. Thanks for doing this; this is a great start. Comment at: docs/LanguageExtensions.rst:1720 @@ -1712,1 +1719,3 @@ +being stored there, and the function returns 1. The behavior of these builtins +is well-defined for all argument values.