If the maps are typically small, would SmallDenseMap make sense?

Landed as a0aed80b22d

Update for comments.

Use static

Add static

Remove unit test for ThreadLocal as well.

Getting the repository right.

In D20071#424886, @arsenm wrote:

In D20071#424843, @nhaehnle wrote:

LGTM

Out of curiosity, what does making v_readlane/v_writelane convergent fix? I thought they were independent of control flow...

Now I'm not really sure about them. I was just thinking any of the instructions that do any kind of crosslane interactions would be convergent

LGTM.

I'm nervous about this change. The input code is denorm preserving on architectures that flush denorms, but the output may not be.

I agree with Steve here. Even code that is built with -ffast-math has to live in a reality where NaNs, Infs, etc. are sometimes generated by external function calls, provided as user input, etc. It's critical that this code be able to filter out these invalid values, precisely because the body of the code will be optimized on the assumption that those values did not need to be considered.

In D18670#389991, @escha wrote:

It feels wrong to bloat the commandline options like this, I guess

In D18670#389959, @evstupac wrote:

But now in LLVM trunk there is no way to make FullUnrollMaxCount 6. Right?
To make FullUnrollMaxCount introduction reasonable there should be a path in LLVM trunk where FullUnrollMaxCount can become something else than UINT_MAX. That will resolve the issue with test as well.

LGTM

LGTM

Needs doxygen comment.

Code change is fine, but I would like to see a doxygen comment for scalarizeVectorLoad()

Hi Lang,

LGTM.

r262376

Ping?

Added new CreateWithCopiedFlags() on BinaryOperator.

Ok, I see what you're trying to get at, and I agree with your assessment that a it is removing a control dependence. Since it's specific to exactly the transform being applied, I think a little deeper explanation in the comment is merited.

I don't agree with the new comments. If conversion does not remove any dependencies. It just changes their expression from control flow to predication, but they're still control dependencies in that they gate execution.

I have convergent instructions that are essentially register->register arithmetic operations. I don't have any great dependence on them being rematerialized, but I wanted to point out that it's not inconceivable.

If-conversion doesn't change the control dependencies of the MIs that it moves. It just changes the expression of the dependency from expression via branches to expression via predication. Either way, the execution of the convergent op is still gated by the same set of expressions. Hence, I don't think any change is needed here.

Why is this needed?

My targets have convergent instructions that are rematerializable.

LGTM.

Do you have any suggestions on how a target might make a reasonable choice for the limit? Does the limit trend down as the size of the register file goes up?

In D17141#351645, @rengolin wrote:

In D17141#351607, @resistor wrote:

It *does* depend on the OS running underneath, and I'm providing an example right here. This is comparable to the fact that many implementations do not provide sNaN support.

I'm not sure I understand. Are you talking about Apple's implementation of the SIMD unit, or about the OS setting up some different flags?

AFAIK, you can't "turn on" IEEE compliance on NEON with hardware flags. So, if you accept non-IEEE-compliant code in Darwin (the OS?) becomes a platform decision, not a hardware requirement. This flag is about the hardware requirement.

In D17141#351600, @rengolin wrote:

In D17141#351589, @resistor wrote:

That's not correct for all platforms. Darwin, for example, uses NEON for floating point even in non-fast-math mode. The platform is simply defined not to support subnormals.

Well, NEON denormal behaviour is independent of the OS running underneath, so this is the correct information. But it may not be the best decision for Darwin, and that's a separate story.

That's not correct for all platforms. Darwin, for example, uses NEON for floating point even in non-fast-math mode. The platform is simply defined not to support subnormals.

That makes sense to me in principle, but I know nothing of why CWD is needed for (some form of?) DWARF generation at all.

LGTM.

LGTM.

LGTM.

Functionality LGTM. I would add a comment at the use site of TokenFactorResult to point out that the walk is being memoized.

I made an attempt to remove ownership from CompilerInstance, but ASTUnit has a massively complex relationship with FileManager, and foiled my attempt.

I am not comfortable authoring a patch to do that, as I don't have a clear understanding of the clients involved, and have no means to verify its correctness.

! In D15918#320308, @klimek wrote:

I understand - my question is, why not instead take away ownership from CompilerInstance?

In D15918#320300, @klimek wrote:

Thx for fixing. What's the reason that the CompilerInstance wants ownership?

Your proposed fix seems conservatively correct to me. I'd recommend getting a review from one of the ARM maintainers, perhaps tnorthover?

In D12345#288471, @dberlin wrote:

The last status i saw was: "I'll try seeing what little needs to be done to
converge in the couple cases that I have (one above) and a superficial look
at why the second reassociate improves codegen."

I saw no update on that ;-)

Ping? Was there any further feedback on this? We're seeing pretty horrible regressions caused by this.

Patch LGTM. Testcase?

Patch LGTM. Testcase?

Patch looks LGTM. Is it possible to provide a test case, maybe for AMDGPU?