llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.43k stars 11.74k forks source link

missed fold, failed to kill redundant shufflevector #86079

Open zhengyang92 opened 6 months ago

zhengyang92 commented 6 months ago

https://godbolt.org/z/T4sTh8W87 https://alive2.llvm.org/ce/z/fh2VmQ

define <2 x float> @src(<2 x float> %0, i8 %1) {
if.else433:
  %2 = shufflevector <2 x float> %0, <2 x float> poison, <2 x i32> <i32 1, i32 poison>
  %3 = fneg <2 x float> %0
  %4 = shufflevector <2 x float> %2, <2 x float> %3, <2 x i32> <i32 0, i32 3>
  ret <2 x float> %4
}

define <2 x float> @tgt(<2 x float> %0, i8 %1) {
if.else433:
  %2 = fneg <2 x float> %0
  %sv = shufflevector <2 x float> %0, <2 x float> %2, <2 x i32> <i32 1, i32 3>
  ret <2 x float> %sv
}

@dtcxzyw @regehr @RKSimon

dtcxzyw commented 6 months ago

Similar issue: https://github.com/llvm/llvm-project/pull/84645

It may be a SLP issue. @zhengyang92 Could you please provide the IR before SLP?

cc @alexey-bataev

RKSimon commented 6 months ago

Instcombine already does something similar for binops in InstCombinerImpl::foldVectorBinop, so we probably need to try to add an InstCombinerImpl::foldVectorUnaryop equivalent

llvmbot commented 6 months ago

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. In the comments of the issue, request for it to be assigned to you.
  2. Fix the issue locally.
  3. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  4. Create a Git commit.
  5. Run git clang-format HEAD~1 to format your changes.
  6. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

llvmbot commented 6 months ago

@llvm/issue-subscribers-good-first-issue

Author: Zhengyang Liu (zhengyang92)

https://godbolt.org/z/T4sTh8W87 https://alive2.llvm.org/ce/z/fh2VmQ ``` define <2 x float> @src(<2 x float> %0, i8 %1) { if.else433: %2 = shufflevector <2 x float> %0, <2 x float> poison, <2 x i32> <i32 1, i32 poison> %3 = fneg <2 x float> %0 %4 = shufflevector <2 x float> %2, <2 x float> %3, <2 x i32> <i32 0, i32 3> ret <2 x float> %4 } define <2 x float> @tgt(<2 x float> %0, i8 %1) { if.else433: %2 = fneg <2 x float> %0 %sv = shufflevector <2 x float> %0, <2 x float> %2, <2 x i32> <i32 1, i32 3> ret <2 x float> %sv } ``` @dtcxzyw @regehr @RKSimon
aniplcc commented 6 months ago

Hi, I'm new to instcombine. Am I right for thinking this implies the following:

  1. implement foldVectorUnaryop(UnaryOperator &Inst).
  2. in /llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp, for visitFNeg, add the new foldVectorUnaryop( call. I'm interested in implementing this.
dtcxzyw commented 6 months ago

%2 = shufflevector <2 x float> %0, <2 x float> poison, <2 x i32> <i32 1, i32 poison> %4 = shufflevector <2 x float> %2, <2 x float> %3, <2 x i32> <i32 0, i32 3> %sv = shufflevector <2 x float> %0, <2 x float> %2, <2 x i32> <i32 1, i32 3>

@RKSimon Is it safe to change the mask?

dbelow02 commented 6 months ago

Hey there! I'm new to llvm and I'd like for this task to be assigned to me.

zxc12523 commented 6 months ago

@RKSimon Could you explain more on this topic? I don't understand the relation of this optimization with InstCombinerImpl::foldVectorUnaryop, or does this optimization related to https://github.com/llvm/llvm-project/blob/ed1cfffe9b2b2d3cc9279ff83400ace156b317a2/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp#L2454

RKSimon commented 6 months ago

Sorry - you're better off with somebody who understands all the InstCombine folds, there might be something already present that almost does this that I'm not aware of

madhur13490 commented 3 months ago

Is anybody still looking at this?

juanarbol commented 2 weeks ago

I could take a look; I'll try to work on this.

elchukc commented 1 week ago

@juanarbol are you still working on this? If not I'd like to take it up.