Rebase

Remove getDefaultConstantForOpcode function for now.

test update to simpler

remove extra flag -slp-vectorizer in test.

update testcase.

update after RKSimon's remarks.

Rebase

correct test filename spelling.

update test.

Update after RKSimon's remarks and replaced testcase for more appropriate.

update after RKSimon remarks and add test.

Test cases?

Well, I could add tests here, but it is a like a main flow of this algorithm and it should be tested already here.

Remove technical comments

Fixed issue with horizontal-list.ll.

Oops, I just noticed that we have lost flag in horizontal-list.ll , I will look at the issue.

Rebase

Rebase, remove all "End function" lines.

Reversed back the last change.

update after Craig's remark.

Fix "change to a subregister" issue pointed by Craig.

Update to vector-shuffle-combining-avx.ll

update with X86::AVX512_256_SET0 included, please ignore my last comment

we don't need to change X86::AVX512_256_SET0, it is already using X86::sub_ymm there, I have not seen any ZMM to YMM change in the diff.

ok, thanks, I will redo the change.

Update after RKSimon's remarks.

We decided to split this change into two AVX and AVX512 , this is part one AVX and AVX2.

updated manually vec_uint_to_fp-fastmath.ll, memset.ll

and memset.ll, sorry.

oh, I missed that vec_uint_to_fp-fastmath.ll should be updated manually. I will redo my change.

add X86::AVX512_256_SET0 handling, format, rebase

oh, I missed "X86::AVX512_256_SET0:" case , I have to check some testcases, I think I saw there something incorrect. I will redo my change.

Ping.

rebase after recent changes.

rebase

rebase to the current tree.

update after rksimon's remarks.

Fixed issue with test/Transforms/SLPVectorizer/X86/horizontal-list.ll, Merged two versions of propagateIRFlags into one.

Resulting in to:
VL[0]: %r1 = add i32 %arg, undef
VL[1]: %r2 = add nsw i32 %r1, undef
VL[2]: %r3 = add nsw i32 %r2, undef
VL[3]: %r4 = add nsw i32 %r3, undef
VL[4]: %r5 = add nsw i32 %r4, undef

I'm not sure if Z having those flags is on purpose, though. And I don't see what we get from that.

Please tell me if I misunderstood something. I'm not the most familiar with the SLP vectorizer.

Usually it should have flags in common and it collects those flags see "Intersection->andIRFlags(V);" in the loop across all VL,
but I have one example from test/Transforms/SLPVectorizer/X86/horizontal-list.ll where NUW flag was canceled by NSW.
VL[0]: %r1 = add nuw i32 %arg, undef
VL[1]: %r2 = add nsw i32 %r1, undef
VL[2]: %r3 = add nsw i32 %r2, undef
VL[3]: %r4 = add nsw i32 %r3, undef
VL[4]: %r5 = add nsw i32 %r4, undef

I don't get what you want to do here, sorry.
As I see it, in all the uses of propagateIRFlagsWithOp(X, Y, Z), we have Z == Y[0]. Which will end up being *almost* the same as the propagateIRFlags but with the side-effect that Z will also have those flags.

propagateIRFlagsWithOp only collects flags from Y if Y.Code == Z.Code while propagateIRFlags copies any flag from Y.

Abandoning review due to : there should separate reviews for one for InstCombine Pass and another one for SLP, and dereferenceable_or_null metadata can only be applied to loads of a pointer type.

update after D34756 commit.

update after rksimon's remarks.

Update formatting, comments

Update after http://lists.llvm.org/pipermail/llvm-dev/2017-June/114120.html. Added 0x1b(_CMP_FALSE_OS), 0x1f(_CMP_TRUE_US) handling.

Ping. [andrew.w.kaylor, scanon] Is it OK to assume that FP exceptions are off by default and allow such transformation to constants in the IR since we know that we would have exception with "1.00 -nan" for _mm256_cmp_ps(a, b, 15) in case FP exceptions are enabled?

We should've asked this first: is that fold allowed in the default FPENV state that we assume that clang is operating in?

I suppose it is FE_ALL_EXCEPT.

Abandoning this revision with there is already open https://reviews.llvm.org/D28907 for the issue.

rebased the change against trunk with no new regressions

We agreed with Alexey that I would continue on this issue:
<dinar_> Hi Alexey, I rebased your change for PR30787, do you mind if I update the change in https://reviews.llvm.org/D28907?
<ABataev> Ok, go ahead

Should we handle the 'pd256' version the same way?
How about the 0xb ('false') constant? It should produce a zero here?
Can or should we deal with the signalling versions (0x1b, 0x1f) too?

hm looks like 0xb(_CMP_FALSE_OQ) is ordered, so it is not possible and 0x1b or 0x1f might emit a signal.

add _mm256_cmp_pd double version
add comments in lib/CodeGen/CGBuiltin.cpp
replaced 0xf to _CMP_TRUE_UQ in avx-builtins.c

canceling because x86 is not currently set up to look at breaking a dependency for anything but the 2nd operand.

updated tests after review with proper update_llc_test_checks.py.

Rebased and changed OptForSize to OptForMinSize.

Fixed some typos.

another update after review. Erased attributes for the test added test's explanation. Added an assert that LCSSAPhi->getIncomingValue(0) is loop-invariant in the original loop

update after review.

yes, both examples from PR32935, PR32859 are fixed by this change

Also, can you please make sure we don't miscompile any of the dups of the original bug with this patch?

you mean PRPR14725's testcase? yes this one looks correct to me after this change.

I think in the particular case we are limited here to just one predecessor, there is "LCSSAPhi->getNumIncomingValues() == 1" condition above.

Well, Out of the LCSSA for we could have "phi i32 [ 0, %for.inc.2.i ]" or "phi i32 [ undef, %for.inc.2.i ]" like in PR14725, but the IR Verifier requires for PHI one entry for each predecessor of its parent basic block. The original PR14725 just added 'undef' for an predecessor BB and it is not correct. We copy the real value for another predecessor instead of bringing 'undef'.

update after issue -Os was cleared.

another update after issue with -Os.

update after review process.

Further changes after rebase.

Some variable name changes.

update after Simon's comments.

here is test-case:
define <4 x double> @cmp256_domain(<4 x double> %a) {

%cmp = fcmp oeq <4 x double> zeroinitializer, zeroinitializer
%sext = sext <4 x i1> %cmp to <4 x i64>
%mask = bitcast <4 x i64> %sext to <4 x double>
%add = fadd <4 x double> %a, %mask
ret <4 x double> %add

}
So, we are changing "immAllOnesV" for 256-bit vectors only for AVX1 machine.

update test again with utils/update_llc_test_checks.py

added update to tests.

update again, something went wrong during the last update.

updated the change after rebase.

update after Simon's comments.

here we are checking that the store depends on the extractelement operation.