This is needed for OpenMP as well. Does it make sense to include it in this patch or in another one?

I'm unsure about the scope of this. It seems to match a particular pattern and it is unclear this is the right place to do so. Have you considered doing this as part of AAPrivatizablePtr (or a new AbstractAttribute) in the Attributor?

One last question

Apologies for my delay.

I'll check later but please add my examples as tests.

I don't like the way you say "non-frozen undef" but maybe it is just me. For me, a frozen value is neither undef nor poison, full stop.
I also don't like "expression containing undef" which could be interpreted as and i1 false, undef, though that one is an OK branch condition.

I'm also not sure about changing Use, @nikic suggestion with marking it in the callinst sounds interesting.

Minor comments, otherwise LGTM.

LGTM. Thanks.

I'm in favor of adding the test (and changing the default value). We have a clear path to re-enable it and it is not far away either.

Allow mem2reg to replace an Alloca used as a nocapture and inaccessiblememonly/readnone function call operand with undef

In D76894#1946810, @ddcc wrote:

Sure, I'm working on a instrumentation pass which is inserting calls that inhibit optimization, so I'm trying to work around the issue using function attributes, and need to look into memory to register promotion next.

In D76673#1946852, @rnk wrote:

In D76673#1945275, @jdoerfert wrote:

In D76673#1945260, @rnk wrote:

lgtm

I think the current verifier check is overly conservative. I think the align argument attribute is only an ABI-relevant attribute when it is used with byval. In your test case, it seems to refer to the alignment of the pointee of a pointer argument, not the alignment of the argument itself. In that case, it is not ABI relevant, and we could tolerate the mismatch.

However, I just don't think it's worth worrying about this detail in attributor. Mainly, it points out that align is overloaded. =(

I don't think in my test it is the pointee of the pointer argument. The load from %p has an alignment of 32, so %p is assumed to be align 32 at this location. Then the argument of musttail_callee_1 is assumed to be align 32 as well but we do not manifest it.
What am I getting wrong?

Sorry, I'm not being clear. Suppose an argument is passed in memory. What is the alignment of that memory? When the attributes byval(<ty>) align N are used, it indicates the alignment of the argument memory.

From the perspective of LLVM IR, byval arguments are represented with a pointer SSA value, so the align attribute means almost the same thing: it means some number of low bits of this pointer are zero.

But from the perspective of call lowering, the align attribute can either be a statement about the bits of the argument value, or a statement about how to lay out arguments in memory on the stack, depending on the presence of other attributes. I tend to see things from this perspective, not the perspective of an IR optimizer. :)

Remove OpenMP from clang/lib/Headers/__clang_cuda_math_forward_declares.h

Add and repair tests to show new features, e.g. math macro support

Thanks for working on this, I'm really hoping we start to use attributes more aggressively. I inlined comments.

I'm fine with this. @reames should probably say something.

I'm fine with disabling this for now. The IRBuilder create alignment as operand bundle patch is already somewhere but not merged (under an option).
We should measure that solution and then either enable this again by default or determine what else is needed.
FWIW, I expect us to need a pass that merges and eliminates llvm.assume with operand bundles soon, that should drive compile time down again for cases where we emit equivalent or subsuming assumptions.

In D76673#1945260, @rnk wrote:

lgtm

I think the current verifier check is overly conservative. I think the align argument attribute is only an ABI-relevant attribute when it is used with byval. In your test case, it seems to refer to the alignment of the pointee of a pointer argument, not the alignment of the argument itself. In that case, it is not ABI relevant, and we could tolerate the mismatch.

However, I just don't think it's worth worrying about this detail in attributor. Mainly, it points out that align is overloaded. =(

This will have an effect on the tests in llvm/tests/Other, please update them accordingly

In D76871#1944849, @lebedev.ri wrote:

I think the patch description is lacking - the switch already exists, and it works.
There should be an explanation as to why it is being moved.

Improve documentation

Two more minor comments, other than that the code looks good. Please update so I can commit it.

I only have coding style comments left. Providing the logic in a more general way will be future work.

LGTM

Split off NFC namespace stuff

pretty print variant names as "variant[device={kind(cpu)}]"

hide the mangling consistently from the user (and fix rebase bug)

[reverse ping] Is this still active?

I love the progress :) Some comments below.

In D75384#1933294, @lebedev.ri wrote:

Hmm. Random question: are these optimizations friendly to the openmp tooling?

In D76673#1941116, @uenoku wrote:

Good point. It is about:
Attribute::StructRet, Attribute::ByVal, Attribute::InAlloca, Attribute::InReg, Attribute::Returned, Attribute::SwiftSelf, Attribute::SwiftError, Attribute::Alignment

Seems reasonable but I'm not sure why we need to care about returned.

Rebase and address comments

Derive but do not manifest alignment and returned for (call site) arguments

In D76673#1938580, @lebedev.ri wrote:

How do we know this is only about alignment?

LGTM.

In D76674#1938566, @lebedev.ri wrote:

In D76674#1938558, @jdoerfert wrote:

In D76674#1938540, @lebedev.ri wrote:

That's the patch i showed, yes :)
I'm not sure this is sound.
Given underaligned load, we can't just take max(align of load, abi alignment)

Hm, so we can only look at the abi if the load alignment is unspecified?

I strongly believe so, otherwise there would be no point in specifying an alignment
smaller than the abi alignment on load/store, and no way to do underaligned loads/stores,
which is obviously not the case.

Repsect underaligned accesses, already tested in Transforms/Attributor/ArgumentPromotion/alignment.ll

Simplify helper code

In D76674#1938540, @lebedev.ri wrote:

That's the patch i showed, yes :)
I'm not sure this is sound.
Given underaligned load, we can't just take max(align of load, abi alignment)

We should finish this review and then focus on the next steps. I added more comments but I think the general logic is fine.

@spatel does this look better?
@reames you requested changes, we'll wait for you.

LGTM.

In D76058#1937554, @pcc wrote:

This change broke the msan buildbot: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/39755

==70358==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5a602b2 in deleteParallelRegions /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Transforms/IPO/OpenMPOpt.cpp:127:9
    #1 0x5a602b2 in (anonymous namespace)::OpenMPOpt::run() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Transforms/IPO/OpenMPOpt.cpp:116:16
    #2 0x5a94252 in (anonymous namespace)::OpenMPOptLegacyPass::runOnSCC(llvm::CallGraphSCC&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Transforms/IPO/OpenMPOpt.cpp:559:19
    #3 0x3f53893 in RunPassOnSCC /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Analysis/CallGraphSCCPass.cpp:139:23

In D76588#1937614, @sstefan1 wrote:

It is a good idea to generalise the way we write tests.

we now use the test update scripts for (almost) all Attributor tests.

What is the problem with those that are not included in this patch? I see noreturn_async/sync are not here

In D76184#1927159, @rnk wrote:

I patched this in with it's dependency, and to compile just Attr.h by itself, it reduces the time from ~2.420s to ~1.920s, so it is worth pushing this through. :)

However, I noticed that there are still some targets broken by the removal of the SmallSet.h include. I'm happy to push that through if you want to hand it off.

In D76184#1927159, @rnk wrote:

I patched this in with it's dependency, and to compile just Attr.h by itself, it reduces the time from ~2.420s to ~1.920s, so it is worth pushing this through. :)

However, I noticed that there are still some targets broken by the removal of the SmallSet.h include. I'm happy to push that through if you want to hand it off.

Could this be the reason for the broken bot? http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/36387
It looks the most likely to me but you say it's "NFC-ish".

Broke the buildbots http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/64546