Of course = ).

Michael

On Jun 20, 2013, at 3:06 PM, Eli Friedman <[email protected]> wrote:

> On Thu, Jun 20, 2013 at 3:05 PM, Eli Friedman <[email protected]> wrote:
> On Thu, Jun 20, 2013 at 2:39 PM, Michael Gottesman <[email protected]> 
> wrote:
> 
> On Jun 20, 2013, at 2:16 PM, Eli Friedman <[email protected]> wrote:
> 
>> On Tue, Jun 18, 2013 at 3:11 PM, Michael Gottesman <[email protected]> 
>> wrote:
>> Hello cfe commits!
>> 
>> The attached patch adds checked-arithmetic builtins to ameliorate such code 
>> in security critical applications (for instance webkit). It simply exposes 
>> {u,s}{add,sub,mul}.
>> 
>> *NOTE* The u{add,sub} overlaps with the multi precision built-ins. I decided 
>> to add in the additional builtin since users are going to see s{add,sub} and 
>> look for u{add,sub}. We could add in a Builtins.h header where I could 
>> implement the checked arithmetic with the multi precision arithmetic 
>> builtins but I felt that was a bigger change than this.
>> 
>> 
>> It looks like you copy-pasted some stuff you didn't mean to in the changes 
>> to LanguageExtensions.rst.
> 
> = /. Fixed.
> 
>> 
>> +    return RValue::get(Builder.CreateZExt(Carry, X->getType()));
>> 
>> These builtins all return bool, right?  Why are you zero-extending here?  
>> (You might be able to get away with this in C because of integer promotions, 
>> but I'm pretty sure it'll explode in C++.)
> 
> = /. Fixed. The reason I was zero extending was that I was basing this off of 
> the multiprecision builtin code and forgot to remove it. 
> 
>> 
>> IIRC, we don't actually support CodeGen of llvm.smul.with.overflow with 
>> 64-bit operands on x86-32; it would be nice to print a proper error message 
>> instead of crashing.  (At least, we didn't at one point; I don't recall if 
>> it ever got fixed.)
> 
> I just codegened my test file on OS X with -arch i386 and it codegened fine. 
> So it looks like it was fixed.
> 
> Hows this look:
> 
> 
> LGTM.
> 
> 
> Oh, one more thing I forgot: please put a note into the release notes.
> 
> -Eli

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to