> > On Fri, 1 Nov 2024, Jan Hubicka wrote: > > > > I have a vague memory that one of the tests in SPEC has a loop that > > > tries to malloc, doubling the size each time, until it fails. Would > > > the patch change the behavior of such a loop? > > > > If the resulting allocation is unused except for NULL check we will make > > it always "succeed" and thus the loop will likely loop forever. I > > wonder what this loop computes on systems, like linux, that overcommits > > memory. > > Even with overcommit, malloc is going to return NULL as soon as you pass > a half or more of the address space as the requested allocation size. > > Since the patch eliminates malloc with unknown size, I think this transform > is incorrect (wasn't correct for the same reason in LLVM either). > > I'm pretty sure it's going to break some Glibc tests that verify that > malloc(SIZE_MAX / 2 + 1) does not succeed.
Is malloc required to return NULL for block of half of address size or it is glibc behavior? Some time ago we was discussing possibility to optimize out a check that std::vector size is already allocated as bigger than 2^63 and conclusion was that this is not safe assumption. I was told that mainframes had some problem by allowing 2^31 allocations in 32bit mode. Instead of wiring in return value to be constant 1 I can wire in condition "retval = size >= half_of_address_space". It is extra work, but possible. I think we may want to have -fno-malloc-dce to use in code like glibc checking where optimization is not intended. Glibc can also use -O0 or -fno-tree-dce if it really does not want the compiler to tamper with malloc/free calls for purpose of testing. > > Please reconsider? Why to we need to match LLVM here? Matching llvm is not really the goal. I implemented it since it is useful optimization for code that builds small objects on heap and compiler can optimize away their use. This is relatively common for code with higer abstraction penalty. My original motivation was std::vector and std::string consumers, but I think there is C code doing similar things, too, so it would be better to support both C and C++ allocations. Honza > > Alexander
