I wonder if we should prefer to widen <2 x i8> and <4 x i8> to <8 x i8> instead of promoting to <4 x i16>. It would make stores like this a bit cheaper. Maybe an interesting experiment at some point (mostly just modifying AArch64TargetLowering::getPreferredVectorAction, I think, and seeing what happens to the generated code).

In D48085#1136910, @lebedev.ri wrote:

In D48085#1136900, @spatel wrote:

In D48085#1136860, @lebedev.ri wrote:

Should this adjust the ReleaseNotes?

The docs provide extra warning/workaround, and this change doesn't affect that language IMO (there's less chance we're going to break code, but we don't have to disclose that).

For reference, here are links to the docs that we added with the previous patches (last updated with D46236 I think):
https://clang.llvm.org/docs/ReleaseNotes.html#new-compiler-flags
https://clang.llvm.org/docs/UsersManual.html#controlling-code-generation
http://llvm.org/docs/ReleaseNotes.html#non-comprehensive-list-of-changes-in-this-release

Let me know if you see anything that can be improved.

The text in first and last links suggests that such an optimization is always being done.
But now it will only be done in presence of no-signed-zeros-fp-math attribute.
Which is controlled by -ffast-math (or maybe some more fine-grained option, too?)

Fixed test case command.

• Generalized test case a bit more and no longer checking only for 0.

In D48324#1136921, @mikhail.ramalho wrote:

I think the correct type should always be ptrdiff_t, regardless of the type of the pointers involved. ASTContext has a method has a getPointerDiffType() so maybe we should you that instead of Ctx.getIntTypeForBitwidth(Ctx.getTypeSize(LTy), true)?

In D48324#1136886, @george.karpenkov wrote:

Changing it to:

Shouldn't we then use maximum of two type sizes? [I'm not sure what are the exact conversion rules there]

In D48085#1136900, @spatel wrote:

In D48085#1136860, @lebedev.ri wrote:

Should this adjust the ReleaseNotes?

The docs provide extra warning/workaround, and this change doesn't affect that language IMO (there's less chance we're going to break code, but we don't have to disclose that).

For reference, here are links to the docs that we added with the previous patches (last updated with D46236 I think):
https://clang.llvm.org/docs/ReleaseNotes.html#new-compiler-flags
https://clang.llvm.org/docs/UsersManual.html#controlling-code-generation
http://llvm.org/docs/ReleaseNotes.html#non-comprehensive-list-of-changes-in-this-release

Let me know if you see anything that can be improved.

LGTM

Patch updated:
Added a 'TODO' comment about using a FABS-based sequence if we can't ignore -0.0.

In D48085#1136860, @lebedev.ri wrote:

Should this adjust the ReleaseNotes?

Fast-isel tests were added for previous codegen in r335068 and updated for new codegen in r335071.

Add RUN: clause to the test

LGTM

Changing it to:

In D48324#1136862, @ddcc wrote:

The code already tries to handle ptrdiff_t specially, see the comment right above, but it is only executed if RetTy is non-null. Instead, I think it should be refactored so that e.g. a local isBool is always computed, and this is passed to the final call to Z3Expr::fromBinOp.

I tested all parts of the Iterator Checkers, all tests passed.

It should be possible to implement this sort of coercion with an appropriate cast, rather than just bailing out, I think. But I won't block the patch on that.

Ok the register class/feature stuff looks good to me now.