[PATCH] D153507: [SLP] Use vector types for cmp alt instructions costs

Fri Jun 23 09:42:03 PDT 2023

vporpo added a comment.

> Yeah I have not committed the updated tests yet. It is common practice to split them out to show just the differences, and I tend to keep them locally until the patch gets further into a review in case they need more adjustment. I can commit them if it's useful, but you can grab the new file using the View Options -> Show Raw File (Right) button. Or do you mean the code in the SLPVectorizer? I think that sound apply to a recent checkout.

I meant to say the test hunk, sorry about that. You don't need to commit the first part, but it sometimes helps if  it is shared on phabricator. In this case I was a bit confused as I could not tell what changes were made in the test compared to the existing version. In particular, I could not see the original `@test` function, but I could see a function named `@degenerate` which looked very much like `@test`, but it was not immediately obvious if only the its name was changed or if there were changes in its body.

================
Comment at: llvm/test/Transforms/SLPVectorizer/AArch64/extracts-from-scalarizable-vector.ll:2
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
 ; RUN: opt -S -passes=slp-vectorizer -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck %s

----------------
dmgreen wrote:
> vporpo wrote:
> > Perhaps set the -slp-threshold such that it gets vectorized?
> The idea is that these shouldn't be vectorized. The degenerate case would get simplified by instsimplify to ret 0, so is not interesting. The with_inputs case doesn't gain anything from vectorization (the f128 fcmp's will just be scalarized, so the vectorized version has are twice as many fcmp, plus extra shuffles and the reduce).
I understand that this can be simplified, but isn't this test checking that SLP matches and generates the `llvm.vector.reduce.and` pattern? Or is there a separate test for that? 

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153507/new/

https://reviews.llvm.org/D153507