* Jason Merrill: > Sorry it's taken so long to review this.
Same here. *sigh* Thanks for your comments. > On 02/21/2011 04:05 PM, Florian Weimer wrote: >> build_operator_new_call (tree fnname, VEC(tree,gc) **args, >> - tree *size, tree *cookie_size, >> + tree *size, tree size_with_cookie, tree >> *cookie_size, > > We don't need both size_with_cookie and cookie_size here. Let's > change the cookie_size parameter to be size_with_cookie instead (and > adjust the places in build_new_1 that check for cookie_size being set > or not). Okay. >> + if (!flag_new_overflow_check) >> + return result; > > Let's check for constant results here. If we have a TREE_CONSTANT > result that overflows, we can handle it even if we aren't emitting the > checks for non-constant values. I assume I can report an error in this case? > And if we have a TREE_CONSTANT result that doesn't overflow, we > don't have to bother building up the complicated trees in order to > fold them away. Understood. This is sort-of needed ... >> + nelts = maybe_build_size_mult_saturated >> + (convert (sizetype, nelts), >> + convert (sizetype, array_type_nelts_top (elt_type)), >> + NULL_TREE); > > It seems unfortunate to do this runtime checking more than once for > multi-dimensional new. Since the element size and all but one of the > array dimensions are going to be constant in most cases, let's try to > multiply the element size with the constant dimensions before we bring > in the potentially non-constant dimension. ... to implement this anyway. I plan to accumulate constant and variable factors separately, and multiply them together in the end. VARIABLE * VARIABLE will still incur an expensive overflow check (involving integer division, even), but there's currently no way around that. I already have something in that direction, but it is still rather messy. Hopefully, it will be a temporary measure only, and this burden can go away when the middle-end learns about saturating arithmetic, or we implement std::bad_array_new_length from C++0X (and have removed VLA support). >> I still need some guidance where to put the test case. There don't >> seem to be any direct tests for operator new[] funcionality. > > Let's put it in g++.dg/init. Okay. Curiously, the test suite has rather poor coverage of operator new[] right now. Multi-dimensional new is also extremely rare in the C++ code bases I looked at.
