We tend to test push .section and .pushsection in this file, so it would be nice to continue doing that.

Can you add a variation with .pushsection for @progbits and @nobits? Otherwise LGTM.

Ping?

This summarizes the IRC discussion.

The original code has a functional dependency between sz and bytes and whether they can be constant evaluated. But the code doesn't express that. I don't think we can enforce that in any sensible way. There are valid use cases after all where partial inlining would result in entirely sensible decisions, just think about the more typical case of __builtin_constant_p selecting between inline asm taking immediate operands and one taking register/memory operands. That's why I am saying that I consider it a lot more useful to provide reliable building blocks to express the dependency and make sure they work.

...in the outer condition, I meant.

Yes, builtin_expect is just noise here. The point I wanted to make is that both sz and bytes should be used in builtin_constant_p in the other condition.

IMO the root of the problem here is that the branches are mixing predicated variables (bytes) with non-predicated variables (sz). If users want deterministic behavior here, that shouldn't be done.

I was discussing this with Bill off-list to get a better idea of the original test case. Basically, with the new constant intrinsic pass, we can provide a stronger semantic: the default LLVM pass chain always constant folds expressions involving is.constant and performs DCE trivially dead branches resulting from the former folding. This means that if the original expression is adjusted from:

if (__builtin_expect(!!(sz >= 0 && sz < bytes), 0)) {
    if (!__builtin_constant_p(bytes))
    ...
}

to the functionally equivalent:

if (!(__builtin_constant_p(sz) && __builtin_constant_p(bytes) && sz >= 0 && sz >= bytes) && __builtin_expect(!!(sz >= 0 && sz < bytes), 0)) {
    if (!__builtin_constant_p(bytes))
    ...
}

then we can actually ensure proper DCE even with -O0 in the default pass chain. That depends over all on two properties:

• If __builtin_constant_p is constant folded by clang, it must DCE the appropiate places.
• the constant intrinsic pass is run

With -O1 or higher, this should work in general.

Committed as 0b999f76575f0196d3cd01c0781b2513b0a1af15 without link.

Require __STDCPP_NEW_ALIGNMENT__ in C++03 mode. Prefer it over max_align_t in a number of tests when allocation alignment is desired. Adjust some tests to do minimal sanity checking of the alignment for C++03 only. Add an explicit check that __STDCPP_NEW_ALIGNMENT__ is more general than max_align_t if both are present.

This looks much better than the in-tree state, thanks.

In D73245#1918287, @ldionne wrote:

In D73245#1918148, @EricWF wrote:

I've already stated my disapproval of this patch. Libc++ has never and will never provide nor value C++03 conformance.
Moving backwards to C++03 is inconsistent with the libraries general direction.

@EricWF makes a point here, we want to move away from C++03.

Ping

It still seems to be a perfectly reasonable optimisation under the semantics of is.constant. The code here seems to be an abuse of it though and makes assumptions that are not sensible.

I'm confused by that example. If jumpthreading makes it possible to answer true, it is no differerent from inlining and other use cases. The reverse is the problematic case, turning constant cases into non-constant cases?

libc++ has no idea what a correct max_align_t is. The internal definition works due to historic requirements that all three fundamental types are supported for new/delete, but we don't have any such guarantees for every other context. A correctly implemented stddef.h does not provide max_align_t in C++03 mode, since that would pollute the global namespace. This means that libc++ currently has two failure modes: on NetBSD, it outright tries to use a non-existing symbol. On other platforms it silently defines max_align_t in a way that can be subtle wrong.

Do not depend on max_align_t in C++03 mode in the test cases.

It is used both in <new> and <memory> and the use in the latter is currently unconditional AFAICT. I don't have a problem splitting the conditional to avoid the typedef. That would address the ODR concern?

Ping2?

Ping?

Let me clarify the situation for a moment:
(1) libc++ does try to work in C++03 mode. See the separate implementation of <functional> for pre-C++11. It is also desirable to support. This is completely beside the question of TR1 support.
(2) The only reason why max_align_t is currently necessary is because it is used as default alignment in <new>.
(3) Most compilers we care about already have a preprocessor symbol specifically for that purpose.

libc++ generally tries to play nice with C++03 code. It doesn't go out of its way to break it and keeping it usable helps dealing with a lot of rusty old code. That's what the patch is all about, not breaking things for no good reason.

Can this be restricted to Linux?

What's the fixup behavior of the linker here, e.g. does it make sense to just use absolute encoding here and let the linker fix it up?

This is normally done by using -Bstatic/-Bdynamic around the library. See tools::addOpenMPRuntime.

Just for the sake of correctness, this is provided by GNU ld as part of the map file (-Wl,-Map=foo.map). It lists all archive members and which object file and symbol triggered the inclusion.

Can we please get this or the other fix in RSN? Mesa hits this.

I'm a bit puzzled by the need for this change. The bump allocator already has logic to do power-of-two scaling for the allocation, so I wonder why it doesn't work properly here.

Adjust based on comments.

Why go back to the large tables for crc32? Just because JamCRC had that bug doesn't mean it should persist.

I wonder if we should actually enumerate evil here, i.e. give the situations in which inlining actually fails. As mentioned on IRC, I wonder if we shouldn't aim for the stronger semantics and at least warn by default of any situation that prevents always_inline from doing its job.

I'd make the argument that it should be strlcpy if present. That covers a lot of existing systems too. I'm not sure this optimisation should be done for freestanding mode though.

2nd ping. Chandler, care to check this please?

Ping.

Ping

LGTM

Can you declare a volatile variable and set that from f instead?

Switch the pass to use two rounds. The first round will collect all relevant intrinsics in RPO, the second one will translate them accordingly.

Apply feedback.

Chandler, are you OK with getting the InstructionSimplify.h part in now, so that it can be merged into 9.0 and the rest follow separately?

It would be interesting to get an actual backtrace. It should not be a DWARF register number, but it might be from a direct caller of the unw interface.

At least for freestanding environments, it would be useful to separate nonnull completely from deferencable. GCC has a separate flag for it, which might also be a reasonable idea.

Hook up a second instance of the pass after the Float To Int pass for optimized builds. This is after the initial loop transforms, so it can profit from some unrolling, but it is before vectorization. The late run of the pass should is kept for now and ensures any potentially added variants are still dropped before SDAG.

It would be nice to also default to this for -ffreestanding.

Simplify the pass logic. First round will update the predecessor links and note if any block is orphaned, a second round will remove unreachable blocks if necessary.
The pass can be further refined to use and update DominatorTree and AssumptionCache incrementally, but this should be functionally complete now. It will not handle more complex cases like orphaned loops, but I don't think those are commonly used with is.constant or objectsize conditions either.
The pass will now scan every BB once, but fall back to the start of a BB of recursive removal invalidates the iterator. This seems to be the strictest form I can manage.

Please read the discussion. The general consensus was and likely is that this is at least somewhat bogus in the standard at best and the cost of not artificially breaking code is much higher than the benefit. Yes, GCC made different choices, but that's no excuse.

Take a look at the list archives for the discussion about attribute nonnull and nullable a while ago. The short version is that the library interface specification might have a wildcard clause that NULL pointers are invalid unless explicitly allowed, but it works perfectly fine on any sensible implementation except the ds9k. When the combination of glibc annotations and aggressive gcc optimizations started, it pointlessly broke a lot of code for no good reason.

Thanks, this looks good enough within the limitations of the framework.

Looking a bit more into the details. Chandler, you've originally suggested going with the LowerAtomic route and that actually does create code that fails the SDAG lowering if the pass is skipped, e.g. on ARM.
The second part is currently overlapping with the CodeGenPrepare pass. I can cleanup the implementation somewhat by reusing the same functionality as that pass is using OR I could factor out a minimal branch for doing the constant folding optimization from CodeGenPrepare as a general branch that is included in the pass chain for -O0, e.g. instead of the more general CodeGenPrepare pass. The main difference is that the non-optimized build would not get any recursive folding from PHI-simplification, but I think that's fine for the original use case. It would also not get the block merging, but again, that seems to be fine for the constraints.

For the first part, I was actually asked to do that. I don't mind either way.

Generalize slightly to also cover llvm.objectsize which has very similar constraints.

The combination of D60942, D06943 and D65280 solves the problems for me on all targets I have.

Update PHI nodes in disconnected block.

Replace boolean argument to replaceAndRecursivelySimplify with optional vector of un-modified instructions. This simplifies the API change significantly and allows other potential use cases. Redo the restart handling. After a successful simplification step, restart the current BB only, but always do another full pass of the function.

Avoid goto. Create new BranchInst instead of modifying in-place. Update tests to reflect changes. Move most of the x86 is-constant test to generic.

LGTM otherwise.