Address latest comments, thanks!

Do not expose getInstructionCost, as the latest version of D109368 can simplify use TTI.

@mkazantsev wrote:
Will it still work if c1 and c2 are checked in the loop?

Update sort order to apply conditions that constraint ranges using max/min (predicates != EQ & != NE).

LGTM, thanks!

Thanks for taking a look! Unfortunately this only worked by accident and I missed that there aren't any tests with the conditions in reverse order.

LGTM as this is needed for correctness.

Interesting test cases, thanks for sharing! Could you update the description to use #55645 instead of just 55645. This ensures the commit will get linked on the GitHub issue page.

Thanks for taking a look!

In D126363#3537438, @simoll wrote:

In D126363#3537416, @ABataev wrote:

We store the the %evl as the related value for VPValue *EVL using State.set(EVL, %evl, Part) and then get required value using State.get(EVL, Part).
In this case we can treat EVL similarly to canonical iv, which is not an invariant.

Ok. If that works then having one global EVL per State defined this way should be fine for us for now.

Rebased on top of the other changes in this area (D122126, D119078). Those patches landed a while ago, so I think now would be a good time to move fowrad with this. Ping :)

Rebase after recent changes.

LGTM, thanks! It would probably be more descriptive to say how the issue is fixed in the commit message, e.g. Skip stores of reduction values before trying to widen or something like that.

In D107966#3533961, @ABataev wrote:

In D107966#3533907, @fhahn wrote:

In D107966#3533879, @ABataev wrote:

Could you check one more time, please?

Yeah I just checked and this crashes for me with a release + assert build (commit is 96323c9f4c10bef5cb5d527970cabc73eab8aa21)

The assertion is: Assertion failed: (II && "Must be an insertelement instruction."), function vectorizeTree, file SLPVectorizer.cpp, line 8543.

Still unable to reproduce but I'll try to investigate it.

is there a number you're seeing which roughly matches performance you were getting with typed pointers?

In D125574#3532385, @Meinersbur wrote:

LGTM. Please wait for @fhahn LGTM as well.

In D107966#3533879, @ABataev wrote:

Could you check one more time, please?

Unfortunately the latest version is still causing crashes when build SPEC2017 on X86. Reproducer below:

Update patch to actually use 100 as limit

Move code to optimize truncates before scalar VF check. Test to check this added in 0c000d77ad09665a0debeb.

In D123537#3530208, @paulwalker-arm wrote:

Hi @fhahn. This patch looks be the cause of the failure reported by https://lab.llvm.org/buildbot/#/builders/176/builds/1723. As well as on AArch64 I also see the failure on X86_64 when doing the following cross compile:

./bin/clang++ -target aarch64-linux-gnu --sysroot=/opt/aarch64-sysroot --gcc-toolchain=/opt/aarch64-cross-compiler -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/Transforms/Scalar -I/home/pmw/projects/upstream-llvm/llvm-project/llvm/lib/Transforms/Scalar -Iinclude -I/home/pmw/projects/upstream-llvm/llvm-project/llvm/include -mcpu=a64fx -msve-vector-bits=512 -mllvm -treat-scalable-fixed-error-as-warning=false -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG    -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT lib/Transforms/Scalar/CMakeFiles/LLVMScalarOpts.dir/SimpleLoopUnswitch.cpp.o -MF lib/Transforms/Scalar/CMakeFiles/LLVMScalarOpts.dir/SimpleLoopUnswitch.cpp.o.d -o SimpleLoopUnswitch.cpp.o -c /home/pmw/projects/upstream-llvm/llvm-project/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

The failure disappears after locally reverting this patch.

It looks like this patch is causing SLPVectorizer to crash with the following IR. This blocks building SPEC on X86, so I'll go ahead and revert this for now to unblock testing.

In D124836#3528109, @vvereschaka wrote:

Hi @void ,

the zero-call-used-regs.ll test gets failed on llvm-clang-x86_64-expensive-checks-ubuntu builder with the following errors:

...
*** Bad machine code: Illegal physical register for instruction ***
- function:    all_arg
- basic block: %bb.0 entry (0x555be568bb88)
- instruction: $q0 = MOVID 0
- operand 0:   $q0
$q0 is not a FPR64 register.
...

Thanks for breaking this up!

LGTM, thanks! Might be good to wait another day with committing in case @reames has additional thoughts.

Address latest comments, thanks!

LGTM, I think that should be fine. ordering by ascending address is likely also slightly more readable for our users.

Thanks!

I think this loss is not only relevant for horizontal reductions, but can pessimize things in general, right?

LGTM, thanks!

LGTM, thanks!

Thanks for the updates!

In D125299#3522290, @nikic wrote:

In D125299#3503151, @fhahn wrote:

Sounds good to me in general. I think there are at least 2 known issues with SimplifyCFG & LoopUnroll introducing branches on poison/undef. Linked to https://github.com/llvm/llvm-project/issues/45489. If you know of other issues in that area it might be good to all link them to that issue, so it is easier to keep track of the outstanding issues

ControlHeightReduction and LoopUnroll have been fixed, patches for IndVarSimplify (https://reviews.llvm.org/D124910), JumpThreading (https://reviews.llvm.org/D125869) and CodeGenPrepare (https://reviews.llvm.org/D125887) are pending.

Address latest comments, thanks!

In D125810#3520608, @bmahjour wrote:

IIRC the reason for calling formLCSSARecursively was to make sure we can handle live outs and reductions properly, which we seem to still be able to do (based on LIT test updates).

Address comments, greatly simplify patch. The new version adds VPExitValues for all values used by LCSSA phis. Non-vplan based helper (fixupIVUsers, fixFirstOrderRecurrence, fixReduction) remove VPExitValues for LCSSA phis they handle directly, before fixing the remaining VPExitValues.

Address comments & aebase before landing.

LGTM as workaround to get the bots back to working.

Just a heads up, we are seeing ~6-7% regressions on some benchmarks with this change. I'll try to narrow down a reproducer.

Rebased before commit.

Thanks for the patch! Makes sense, I added some suggestions with respect to the test.

Still LGTM, thanks! The remaining suggestion can be addressed directly before committing the patch.