I think this patch addresses your review comments. Thanks!
~Aaron On Wed, Aug 27, 2014 at 5:46 PM, Aaron Ballman <[email protected]> wrote: > On Wed, Aug 27, 2014 at 5:41 PM, Reid Kleckner <[email protected]> wrote: >> -// RUN: %clang_cc1 -triple i686-pc-linux-gnu -emit-llvm -o %t-1.ll %s >> -// RUN: FileCheck -check-prefix SANE --input-file=%t-1.ll %s >> -// RUN: %clang_cc1 -triple i686-pc-linux-gnu -emit-llvm >> -fno-assume-sane-operator-new -o %t-2.ll %s >> -// RUN: FileCheck -check-prefix SANENOT --input-file=%t-2.ll %s >> +// RUN: %clang_cc1 -triple i686-pc-linux-gnu -std=c++98 -emit-llvm -o >> %t-1.ll %s >> +// RUN: FileCheck -check-prefix SANE98 --input-file=%t-1.ll %s >> +// RUN: %clang_cc1 -triple i686-pc-linux-gnu -std=c++11 -emit-llvm -o >> %t-2.ll %s >> +// RUN: FileCheck -check-prefix SANE11 --input-file=%t-2.ll %s >> +// RUN: %clang_cc1 -triple i686-pc-win32-msvc -std=c++11 -emit-llvm -o >> %t-3.ll %s >> +// RUN: FileCheck -check-prefix SANE11MS --input-file=%t-3.ll %s >> +// RUN: %clang_cc1 -triple i686-pc-linux-gnu -emit-llvm >> -fno-assume-sane-operator-new -o %t-4.ll %s >> +// RUN: FileCheck -check-prefix SANENOT --input-file=%t-4.ll %s >> >> These RUN lines should use pipes. The less tempfiles we need, the better. > > Can do. > >> >> +// FIXME: There should be a call to generate the std::bad_array_new_length >> +// exception in the Microsoft ABI, however, this is not implemented >> currently. >> +// SANE11MS: [[UWO:%.*]] = call {{.*}} @llvm.umul.with.overflow >> +// SANE11MS-NEXT: [[OVER:%.*]] = extractvalue {{.*}} [[UWO]], 1 >> +// SANE11MS-NEXT: [[SUM:%.*]] = extractvalue {{.*}} [[UWO]], 0 >> +// SANE11MS-NEXT: br i1 [[OVER]], label %[[BAD:.*]], label %[[GOOD:.*]] >> +// SANE11MS: [[BAD]]: >> +// SANE11MS-NEXT: br label %[[GOOD]] >> +// SANE11MS: [[GOOD]]: >> +// SANE11MS-NEXT: call noalias i8* @"\01??_U@YAPAXI@Z"(i32 [[SUM]]) >> >> I think we should keep passing -1 on overflow on Windows in C++11, >> especially given that it's our default language mode in clang-cl. > > Yes, good catch! That was unintentionally changed. > >> >> + if (hasOverflow) { >> + // In C++11 and later, overflow for a call to operator new[] should >> throw >> + // a std::bad_array_new_length exception. >> + if (CGF.getLangOpts().CPlusPlus11) { >> + llvm::BasicBlock *BadArrayNewLengthBlock = >> + CGF.createBasicBlock("new.bad_array_new_length"); >> + llvm::BasicBlock *EndBlock = CGF.createBasicBlock("new.end"); >> + >> + CGF.Builder.CreateCondBr(hasOverflow, BadArrayNewLengthBlock, >> EndBlock); >> + CGF.EmitBlock(BadArrayNewLengthBlock); >> + CGF.CGM.getCXXABI().EmitBadArrayNewLengthCall(CGF); >> + CGF.EmitBlock(EndBlock); >> + } else >> + size = CGF.Builder.CreateSelect( >> + hasOverflow, llvm::Constant::getAllOnesValue(CGF.SizeTy), >> size); >> + } >> >> This is semi-duplicated from above. Here's how I think we can do it with the >> least duplication. Make the CGCXXABI prototype be something like: >> Value *EmitNewArrayLengthOverflowCheck(CodeGenFunction &CGF, bool >> ConstantOverflow, Value *DynamicOverflow, Value *Size); >> >> Then, provide an implementation in CGCXXABI which implements the old >> behavior of returning -1 if ConstantOverflow is true and emitting a select >> i1 on DynamicOverflow with -1. >> >> In ItaniumCXXABI, we can override the method, and in non-C++11 mode, >> delegate back to the base class implementation. In C++11 mode, we can emit >> the basic blocks if we need dynamic overflow checks. MicrosoftCXXABI doesn't >> need to change at all. >> >> Sound good? > > Sounds good, I will come back with an updated patch. > > Thank you! > > ~Aaron
oper_new.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
