[SLP][NFC] Pre-commit test showing deficiency in current roots selection algorithm

[vdmitrie]

Summary

The test case to show not quite optimal SLP vectorization.

[vporpo]

The test filename is vectorize-pair-path.ll, but the description says that this test is about root selection, so perhaps rename it to something like root-selection.ll? The impression I am getting from the current name is that this test is about the path followed by buildTreeRec().

llvm/test/Transforms/SLPVectorizer/X86/vectorize-pair-path.ll

6

Please explain briefly in a comment which roots should be selected instead of the current ones.

7

Nit: perhaps a more relevant function name, like root_selection or something similar.

[vdmitrie]

llvm/test/Transforms/SLPVectorizer/X86/vectorize-pair-path.ll

6

Here we start trying to vectorize reduction but end up in tryToVectorize() method which then makes several attempts of finding something that can be vectorized.
The way it makes probes for various pairs is predefined by its implementation. That is what I mean here by "path".

[vporpo]

LG

llvm/test/Transforms/SLPVectorizer/X86/vectorize-pair-path.ll

6

Sounds good, please add some of this text in the test so that whoever reads it knows what it is about.

[vdmitrie]

Added test description and renamed function as per suggestion.

[vdmitrie]

llvm/test/Transforms/SLPVectorizer/X86/vectorize-pair-path.ll

6

Thank you. I added description to make its purpose clearer.

[RKSimon]

llvm/test/Transforms/SLPVectorizer/X86/vectorize-pair-path.ll

49

Why are there undef float-ops? Is this the only way to expose the issue? If you run this through opt -O3 all you get is a posion result.

[ABataev]

llvm/test/Transforms/SLPVectorizer/X86/vectorize-pair-path.ll

49

It is ok for SLP to have some undefs unless you do not use instcombine. You can treat undef here as a specific op kind, different from others. Just to reduce the size of the test.

[vdmitrie]

llvm/test/Transforms/SLPVectorizer/X86/vectorize-pair-path.ll

49

Yeah, this shouldn't go through instcombine. There were even more undefs after bugpoint reduced the original case so I had to step beck a bit and replaced some with FP constants to make tracing SLP vectorizer behavior easier.

[vdmitrie]

llvm/test/Transforms/SLPVectorizer/X86/vectorize-pair-path.ll

49

typo: back (not "beck")

llvm/test/Transforms/SLPVectorizer/X86/vectorize-pair-path.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -slp-vectorizer -mattr=+avx2 -S | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define double @foo(double %a, double %b, double %c, double %d) local_unnamed_addr #0 {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:    [[TMP1:%.*]] = insertelement <2 x double> poison, double [[A:%.*]], i32 0
+; CHECK-NEXT:    [[TMP2:%.*]] = insertelement <2 x double> [[TMP1]], double [[B:%.*]], i32 1
+; CHECK-NEXT:    [[TMP3:%.*]] = fdiv fast <2 x double> [[TMP2]], <double 7.000000e+00, double 5.000000e+00>
+; CHECK-NEXT:    [[TMP4:%.*]] = extractelement <2 x double> [[TMP3]], i32 1
+; CHECK-NEXT:    [[I11:%.*]] = fmul fast double [[TMP4]], undef
+; CHECK-NEXT:    [[TMP5:%.*]] = fmul fast <2 x double> [[TMP3]], <double 3.000000e+00, double undef>
+; CHECK-NEXT:    [[TMP6:%.*]] = fmul fast <2 x double> undef, [[TMP5]]
+; CHECK-NEXT:    [[TMP7:%.*]] = fsub fast <2 x double> undef, [[TMP5]]
+; CHECK-NEXT:    [[TMP8:%.*]] = shufflevector <2 x double> [[TMP6]], <2 x double> [[TMP7]], <2 x i32> <i32 0, i32 3>
+; CHECK-NEXT:    [[TMP9:%.*]] = insertelement <2 x double> <double undef, double poison>, double [[I11]], i32 1
+; CHECK-NEXT:    [[TMP10:%.*]] = fsub fast <2 x double> [[TMP8]], [[TMP9]]
+; CHECK-NEXT:    [[TMP11:%.*]] = fmul fast <2 x double> [[TMP8]], [[TMP9]]
+; CHECK-NEXT:    [[TMP12:%.*]] = shufflevector <2 x double> [[TMP10]], <2 x double> [[TMP11]], <2 x i32> <i32 0, i32 3>
+; CHECK-NEXT:    [[TMP13:%.*]] = fmul fast <2 x double> [[TMP12]], <double 4.000000e+00, double 1.100000e+01>
+; CHECK-NEXT:    [[TMP14:%.*]] = fsub fast <2 x double> [[TMP12]], <double 4.000000e+00, double 1.100000e+01>
+; CHECK-NEXT:    [[TMP15:%.*]] = shufflevector <2 x double> [[TMP13]], <2 x double> [[TMP14]], <2 x i32> <i32 0, i32 3>
+; CHECK-NEXT:    [[TMP16:%.*]] = fdiv fast <2 x double> [[TMP15]], <double 1.400000e+00, double 1.200000e+01>
+; CHECK-NEXT:    [[TMP17:%.*]] = fmul fast <2 x double> [[TMP15]], <double 1.400000e+00, double 1.200000e+01>
+; CHECK-NEXT:    [[TMP18:%.*]] = shufflevector <2 x double> [[TMP16]], <2 x double> [[TMP17]], <2 x i32> <i32 0, i32 3>
+; CHECK-NEXT:    [[TMP19:%.*]] = fadd fast <2 x double> [[TMP18]], <double undef, double 1.400000e+00>
+; CHECK-NEXT:    [[TMP20:%.*]] = fdiv fast <2 x double> [[TMP18]], <double undef, double 1.400000e+00>
+; CHECK-NEXT:    [[TMP21:%.*]] = shufflevector <2 x double> [[TMP19]], <2 x double> [[TMP20]], <2 x i32> <i32 0, i32 3>
+; CHECK-NEXT:    [[TMP22:%.*]] = extractelement <2 x double> [[TMP21]], i32 0
+; CHECK-NEXT:    [[TMP23:%.*]] = extractelement <2 x double> [[TMP21]], i32 1
+; CHECK-NEXT:    [[I16:%.*]] = fadd fast double [[TMP22]], [[TMP23]]
+; CHECK-NEXT:    [[I17:%.*]] = fadd fast double [[I16]], [[C:%.*]]
+; CHECK-NEXT:    [[I18:%.*]] = fadd fast double [[I17]], [[D:%.*]]
+; CHECK-NEXT:    ret double [[I18]]
+;
+  %i01 = fdiv fast double %a, 7.0
+  %i02 = fmul fast double %i01, 3.0
+  %i03 = fmul fast double undef, %i02
+  %i04 = fsub fast double %i03, undef
+  %i05 = fmul fast double %i04, 4.0
+  %i06 = fdiv fast double %i05, 1.4
+  %i07 = fadd fast double undef, %i06
+  %i08 = fdiv fast double %b, 5.0
+  %i09 = fmul fast double %i08, undef
+  %i10 = fsub fast double undef, %i09
+  %i11 = fmul fast double %i08, undef
+  %i12 = fmul fast double %i10, %i11
+  %i13 = fsub fast double %i12, 11.0
+  %i14 = fmul fast double %i13, 12.0
+  %i15 = fdiv fast double %i14, 1.4
+  %i16 = fadd fast double %i07, %i15
+  %i17 = fadd fast double %i16, %c
+  %i18 = fadd fast double %i17, %d
+  ret double %i18
+}
+
+attributes #0 = { "unsafe-fp-math"="true" }