Thanks for this!

LGTM after two nits are addressed. Thank you!

LGTM after a nit is fixed; thanks!

This revision was not accepted when it landed; it landed in state Needs Review.

(Committed as noted by echristo; just trying to clean my queue a bit. :) )

(Committed as noted by echristo; just trying to clean my queue a bit. :) )

Ping :)

(Sorry for the double-post; one more bit that I noticed scrolling up)

This LGTM after one last ultra-small nitpick is addressed. I don't see you in the commit logs, so will you need me to commit this for you?

Hi, and thanks for this patch!

Rebase now that our LocationSize changes are in

Thanks for the clarification, and sorry about the misunderstanding. I submitted the bit-for-bit NFC patch as r333314, and will rebase the original review soon. :)

(FWIW, Alina and I chatted about this offline. She has a few changes queued up; I'm adding my nits on unchanged bits now, and will do a thorough code review once the new patch is posted)

WFM; thanks for checking!

Thanks!

(Test committed as r332017)

LGTM; thanks again.

Adding George to see if he remembers any cases, so you have some stuff to test this all with.

I don't know this code well enough to LGTM this, but can you talk a bit about why, please? This side-effect-inside-an-assert looks sane to me.

Hi, and thanks for the patch! I'm pretty familiar with this code and am happy to review it when I get back from vacation next week.

Accidentally left in LocationSize::precise. Oops. :)

I am concerned that you doing too many things in one patch and that the mix of refactoring and functional changes may contain hard to spot bugs.

(Thanks for the patch, by the way!)

Ping :)

I plan to commit this tomorrow, to give time for any last-minute comments.

Address feedback

Hi! It fell off my radar, but I'm happy to put it back on my queue. :)

Rebased

Thanks for the feedback!

Addressed feedback

This is a DRAFT API based on what I found needed in the old LoopUnswitch pass.

Thanks for the patch!

Ping :)

Addressed feedback

Addressed feedback

I would wait for another review

C89 has no bool type and no stdbool.h but it has been added to later standards? That means the generalization could theoretically be done for later C standards, because we could check if the bool typedef had been used? If yes, would the check benefit?

Thanks for the feedback!

Addressed feedback

Will be good idea to clarify where TEMP_FAILURE_RETRY come from.

Addressed feedback.

Ping :)

Thanks for the comments!

Addressed comments + tweaked mapTombstone and mapEntry so they call the hacky "direct" constructor.

ping :)

A bundle of nitpicks for you. LGTM assuming those get addressed.

Gentle ping for confirmation from Hal or Philip that we're OK with ignoring lifetime intrinsics (as they exist today) in MSSA. :)

Overall approach looks good to me. Just a few naming/refactoring nits and I'm happy to stamp this. Sorry for taking so long to get back to it

So, we have to support them at the moment because, at the very least, the inliner will merge allocas with non-overlapping lifetimes (see mergeInlinedArrayAllocas in lib/Transforms/IPO/Inliner.cpp). I was surprised when I saw this. :)

+hal and philip, as requested. +gberry in case they have any earlycse thoughts

Only looked at the bit I was mentioned in.

Thanks for this! Just a few nitpicks.

LGTM assuming my nit gets addressed.

Thanks for working on this!

I like that idea. :)

Is it enough to simply replace StringMap with std::map<std::string, ..> for small maps? Do you have memory data with that?

Addressed feedback; thanks!

That said, one of the upsides of the current ubsan is that whether it will produce a diagnostic is predictable (as long as you don't use uninitialized data); you lose that to some extent with llvm.objectsize because it depends on the optimizer.

If the answer to my question is "no, it'll just work," LGTM. Thanks!

LGTM. Thanks again!

Thanks for this!

Thanks again!

Addressed all feedback. Thanks!

However, the tests cover floating point, but they don't cover vector calls (arm_neon.h).

Addressed feedback. Thanks!

Simplify test-case

Thanks for the patch!

I like the idea of fixing those things, too; I'll start poking them soon. :)

please run clang-format. over the changes for better inndentation

Address all feedback

Sorry for taking so long to get back to this, and thanks again for the comments/reviews!

Thanks for the comments!

Address all feedback

Thanks for working on this!

r306467 used __has_extension(overloadable_unmarked) instead.

Totally idiotic to not see them myself to begin with. Sorry and thanks!

LGTM after a few stylistic nits are addressed. Thanks again!

Thanks for this! One nit, and I think it looks good.