Experimentally, this has been exercised, at least by Rust, this week: this change broke our CI, and I think we should revert this change now that I understand the situation.

The input IR below (taken from a test case in the tensorflow repo) optimizes just fine with opt-passes=loop-vectorize from right before this revision, but opt from this revision or later just hangs trying to do the optimization as far as I can tell. I gave it 3 minutes before I killed it. Given that, I'm going to roll this back until you can investigate. Sorry!

This revision appears to have broken some tests inside TensorFlow in a way I'm afraid I don't understand (I've got some TF people digging.) Specifically a handful of tests start timing out after this patch, presumably due to a codegen difference.

Looks obviously correct to me.

TestFrameFormatNameWithArgs.test appears to be broken on CI. Could you follow up with a fix?

In D130107#3684978, @qiongsiwu1 wrote:

May I ask for some guidance/example on how to use the attributes to identify that the removed functions (e.g. malloc, calloc, realloc) are still allocation functions?

Is it true that bool llvm::isAllocationFn returns false now if the input is a malloc call? It seems to be so in my experiments. Is this intended? This is contradictory to the code comment .

Nice catch on the missing allocalign!

This is obsolete due to work in 46e6dd84b778dd2e30368183fec265beceecdd0f

Should be ready. For some reason I can't find some of your comments to mark done, but everything should be done (this is essentially the version you shared with me on zulip)

In rGede600377cb6df1bef71f070130d8cfe734cc5b7#1106840, @durin42 wrote:

This revision appears to be the source of a flaky failure in rustc: https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/12018#018206e2-6a97-41bf-9158-5342ef47b256

I'm not sure what this change is about, but without this patch all tests pass, and with this patch I see flaky failures (exact flake rate seems to depend on the number of cores in the machine: it's about half the time on our modest CI machine, under 5% of the time on my fairly large workstation). I'll try and figure out some ThreadSanitizer for this, I guess, but if this tickles something in your mind please let me know.

This revision appears to be the source of a flaky failure in rustc: https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/12018#018206e2-6a97-41bf-9158-5342ef47b256

In D124425#3582970, @bogner wrote:

This affects tests a bit more than I'd expect - I guess these are test issues where we weren't calling the builtin free for one reason or another. Would it be better to fix up the tests so that they do the right things without this change beforehand to make it obvious that this isn't changing behaviour in a weird way?

In D123090#3564542, @nikic wrote:

In terms of general approach, we'd probably be better off adding support for the attributes in MemoryBuiltins (with a test on a custom allocation function somewhere), and then do the removal from the tables separately (that's also the bit that needs test adjustment).

I've rebased this, and think it's probably still relevant.

@nikic where do I stand on this series? Am I waiting on you or should I try and rustle up another reviewer?

Looks fine to me, but I have no idea what it would take to get the docs published there either. Maybe that's an easy fix?

In D123087#3522318, @nikic wrote:

I think the general preference would be to explicitly add necessary attributes to test cases, rather than running inferattrs. In some of these cases it's not clear to me why the test would need extra attributes, e.g. in case of llvm/test/Transforms/MemCpyOpt/memcpy.ll -- it does test @malloc, but I don't think it needs anything beyond the noalias attribute that's already present.