lgtm, thanks!
On Thu, Aug 28, 2014 at 6:11 AM, Aaron Ballman <[email protected]> wrote: > 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 >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
