CreateAlignedLoad allows align 0, as far as I see in the code.
We just assume that masked.gather is created from scalar load by vectorizer. Vectorizer just copies alignment from scalar load to masked load/gather and the requirement to alignment should match load alignment.

Currently we keep Pointer SCEV type as Index type because we don't have any special SCEV Expression type for pointers.
I suppose, we should add scAddPtrExpr (add index to pointer) and scPtrDiffExpr (pointers diff) to solve this problem. The scAddPtrExpr will always return PonterSizeType and will be expanded to GEP, not to "add". The scPtrDiffExpr will return IndexSizeType and will be expanded to trunc+sub.

ABS is only an example. We'd like to enable/disable the common transformation for other nodes as well.
Is it reasonable to add CombineLevel to the interface to make it more general?

I explained why there's no test. The following commit (linked in the stack) has a test case which crashes without this change.

You've dropped the following commit.

Can you use the "address space" to distinguish between global and local in Data Layout?

I'm ok with the code itself. I agree with Eli that you should add CHECKs.

Add a test, please.

Yes. The scatter operation should be in-order after split.

Ping

Ping.., Can anybody complete the review, please?

Added more tests for BasicAA.

I looked at the code in SeparateConstOffsetFromGEP::canonicalizeArrayIndicesToPointerSize
The DL->getIntPtrType() does the right job for the "custom" GEP. I added a test that covers canonicalizeArrayIndicesToPointerSize().
I'll try to add a test to the constantOffsetHeuristic() as well.

I separated the CodeGen and middle-end changes.
I added one more test for SeparateConstOffsetFromGEP.

In D48323#1137389, @bjope wrote:

AFAIK BasicAA assumes that all GEP indices have a common type. So "normalization" is for example needed before passes like GVN that uses MemoryDependenceResults, that is using BasicAA. Or maybe that is a bug in BasicAA?

Passes like InstCombine might skip doing rewrites in some basic blocks (e.g. if they are unreachable from entry). So I do not think that we can't rely on InstCombine doing a normalization for all GEP:s in the function.
So are we moving towards always using the "normalized" type from scratch (in all passes) when creating a GEP, or what is the plan here?

The new test is auto-generated now.

I wrote this documentation after implementation. I don't work on X86 about a year, but I doubt that somebody touched this code.

I tried to add documentation a while ago https://reviews.llvm.org/D26743

In D43441#1098588, @craig.topper wrote:

Or we'd have to use undef for the upper elts and add a DAG combine to turn insert into undef into a broadcast if its foldable.

I think it is simpler than load folding in td. You can open a bugzilla ticket for possible optimization.

In D46386#1096833, @rjmccall wrote:

The actual semantic parts of the diff seem to have disappeared from the patch posted to Phabricator, for what it's worth.

It is not disappeared by itself, I removed it. I understood that you don't see any added value in the entire memory model description inside.
Thank you.

Added a line about *load-store* semantics of these two intrinsics.
Removed the common description of memory modeling.

Given more clarification about memory model of atomic operations.

Removed the unsigned version of atomics. Enhanced semantics check.
Added more tests.
Added documentation.

In D46386#1087533, @Anastasia wrote:

Is this some sort of a vendor extension then? OpenCL 1.2 atomic builtins don't have ordering parameter.

no context in the patch

Are there MIR changes only? What changes do you expect in the asm code?

Added more tests with custom data layout.

Updated according to the latest comments.

I don't see these tests in this current version of the patch.

All tests that you see *-custom.ll" go through the scev calculations.

@theraven , the latest uploaded version is aligned with what you implemented out of the tree. Could you, please, take a look?

" > We have addressed this by adding explicit PTRADD SelectionDAG nodes, which perform pointer + integer addition. For complex addressing modes, we end up with (ptradd base (some complex integer ops)). This works well as long as the underlying hardware supports address register + integer register addressing, which I presume is the case for Intel (it is for all Harvard architectures that I've come across)."

Yes, we also added ADDPTR node for SelectionDAG and we have more changes related to the special pointer type. Apparently, the codegen does not work with MVT::Ptr.
We can try to upstream the part of DAG builder, that makes ADDPTR from GEP.

Added index width specification to the DataLayout. Updated the langref.
Fixed Pointer vs Index sizes in the code.
Added more tests.

I looked at your code:
/ Return the size in bits of the specified type, for which isSCEVable must
/ return true.
uint64_t ScalarEvolution::getTypeSizeInBits(Type *Ty) const {

assert(isSCEVable(Ty) && "Type is not SCEVable!");
const DataLayout &DL = getDataLayout();
if (PointerType *PT = dyn_cast<PointerType>(Ty))
  return DL.getPointerBaseSizeInBits(PT->getPointerAddressSpace());
return DL.getTypeSizeInBits(Ty);

}
I can't say that size of pointer is smaller that it is. I can't truncate pointer to integer in order to expand all SCEV expressions.

I want to deprecate SCEVs for pointers if the index size is not equal to pointer size.
What do you think?

In D42123#981602, @theraven wrote:

I don't like this patch as is, for several reasons.

1. It's adding a hack that assumes that the offset should be the width of the widest integer operation. This is probably true in most cases (it is for us), but if we're going to introduce the idea that an address offset is distinct from the size of the pointer then we should do it properly and add that to the TargetInfo string explicitly (defaulting to the same size, if not specified).

So you propose to extend Data Layout string and add index size to it, right? It was one of options that Hal suggested. Ok.

Updated, following Craig's comments.

Added tests for a data layout, where pointer is wider than the largest supported integer type.

Fixed 2 "assert" messages.

In D41634#966775, @lebedev.ri wrote:

I understand that if it is a out-of-tree target, the testcase is problematic,
but no testcase at all is worrying. Maybe unit-test can help?

+ minor comment

Re-arranged the code to avoid duplication.
Fixed Debug prints.

Please submit the same test cases before optimization.

I suggest to open a ticket in bugzilla about possible replacement of "select" with constant mask with shuffle to avoid mask instructions on AVX-512.

In this case we need to add "SlowShuffle.." property to these two targets. I don't see any other way to distinguish. Or "Fast" to HSW, but I suggest the fist variant, because less instructions is more obvious.

In D38318#891595, @RKSimon wrote:

@delena @zvi What do you want to do with this. IMO we shouldn't be prematurely combining to variable mask shuffles, and this should be performed later as a scheduler based decision. But that will involve a lot of work that I don't think we're ready for (D26855 tried to move some other code to the MC and hit a lot of issues).

What we could do is add a FeatureFastVariableShuffle feature flag to Haswell and later Intel CPUs and perform the decision in combineX86ShuffleChain off that?