llvm / llvm-project

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

[DAG] Failure to recognise ISD::AVGFLOORS -> ISD::AVGFLOORU fold for non-negative arguments #84746

Open RKSimon opened 8 months ago

RKSimon commented 8 months ago

https://rust.godbolt.org/z/ejh1bh97q

Pulled out of Hacker's Delight - if a signed floor average is between known non-negative arguments then the unsigned variant can be used instead, to potentially allow further simplification in the future.

(Sorry only aarch64 has decent ISD::AVG coverage atm):

define <8 x i16> @uhadd_in_disguise(<8 x i16> %a0, <8 x i16> %a1) {
  %m0 = and <8 x i16> %a0, <i16 510, i16 510, i16 510, i16 510, i16 510, i16 510, i16 510, i16 510>
  %m1 = and <8 x i16> %a1, <i16 510, i16 510, i16 510, i16 510, i16 510, i16 510, i16 510, i16 510>
  %r = call <8 x i16> @llvm.aarch64.neon.shadd.v8i16(<8 x i16> %m0, <8 x i16> %m1)
  ret <8 x i16> %r
}
declare <8 x i16> @llvm.aarch64.neon.shadd.v8i16(<8 x i16>, <8 x i16>)
uhadd_in_disguise:                      // @uhadd_in_disguise
        mov     w8, #510                        // =0x1fe
        dup     v2.8h, w8
        and     v0.16b, v0.16b, v2.16b
        and     v1.16b, v1.16b, v2.16b
        shadd   v0.8h, v0.8h, v1.8h     // <--- FIXME - could be uhadd
        ret

CC @davemgreen

llvmbot commented 8 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 8 months ago

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

Author: Simon Pilgrim (RKSimon)

https://rust.godbolt.org/z/ejh1bh97q Pulled out of Hacker's Delight - if a signed floor average is between known non-negative arguments then the unsigned variant can be used instead, to potentially allow further simplification in the future. (Sorry only aarch64 has decent ISD::AVG coverage atm): ```ll define <8 x i16> @uhadd_in_disguise(<8 x i16> %a0, <8 x i16> %a1) { %m0 = and <8 x i16> %a0, <i16 510, i16 510, i16 510, i16 510, i16 510, i16 510, i16 510, i16 510> %m1 = and <8 x i16> %a1, <i16 510, i16 510, i16 510, i16 510, i16 510, i16 510, i16 510, i16 510> %r = call <8 x i16> @llvm.aarch64.neon.shadd.v8i16(<8 x i16> %m0, <8 x i16> %m1) ret <8 x i16> %r } declare <8 x i16> @llvm.aarch64.neon.shadd.v8i16(<8 x i16>, <8 x i16>) ``` ```asm uhadd_in_disguise: // @uhadd_in_disguise mov w8, #510 // =0x1fe dup v2.8h, w8 and v0.16b, v0.16b, v2.16b and v1.16b, v1.16b, v2.16b shadd v0.8h, v0.8h, v1.8h // <--- FIXME - could be uhadd ret ``` CC @davemgreen
chansuke commented 8 months ago

Hi. I would like to work on this

RKSimon commented 8 months ago

Thanks @chansuke - I think ISD::AVGCEILS -> ISD::AVGCEILU can be performed in the same way.

I'd expect both of these to be handled in DAGCombiner::visitAVG

RKSimon commented 7 months ago

@chansuke reverse-ping - any progress?

chansuke commented 7 months ago

@RKSimon No progress yet.Start working this weekend🙏

RKSimon commented 5 months ago

@chansuke ping?

chansuke commented 5 months ago

@RKSimon I will resume work on this starting tomorrow.

RKSimon commented 4 months ago

You should be able to create very similar patterns to these: https://github.com/llvm/llvm-project/blob/fae31e283203da9a4a3225e2d016e245d4887c2f/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L5280:L5293

chansuke commented 4 months ago

@RKSimon thanks

chansuke commented 4 months ago

@RKSimon I have added the implementation. How can I test it?

RKSimon commented 4 months ago

Running ninja check-llvm-codegen should find any cases that are already committed - we will then need to add additional test coverage, probably in aarch64 and x64 tests