Skip to content

[DAG] TargetLowering::expandABD - investigate alternative expansions #84639

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
RKSimon opened this issue Mar 9, 2024 · 9 comments
Closed

[DAG] TargetLowering::expandABD - investigate alternative expansions #84639

RKSimon opened this issue Mar 9, 2024 · 9 comments
Assignees
Labels
invalid Resolved as invalid, i.e. not a bug llvm:SelectionDAG SelectionDAGISel as well

Comments

@RKSimon
Copy link
Collaborator

RKSimon commented Mar 9, 2024

As mentioned here by @jayfoad #84431 (comment)

Incidentally Hacker's Delight has a section "Average of Two Integers" which has some neat tricks for implementing these operations without extending to N+1-bit integers.

These include alternative code for targets with poor MIN/MAX/SAT/CMPSEL instructions as well as simpler expansions depending upon the knownbits/numsignbits of the operands.

@RKSimon RKSimon added good first issue https://github.com/llvm/llvm-project/contribute llvm:SelectionDAG SelectionDAGISel as well labels Mar 9, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 9, 2024

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
Copy link
Member

llvmbot commented Mar 9, 2024

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

Author: Simon Pilgrim (RKSimon)

As mentioned here by @jayfoad https://github.com//pull/84431#issuecomment-1986043833

> Incidentally Hacker's Delight has a section "Average of Two Integers" which has some neat tricks for implementing these operations without extending to N+1-bit integers.

These include alternative code for targets with poor MIN/MAX/SAT/CMPSEL instructions as well as simpler expansions depending upon the knownbits/numsignbits of the operands.

@Sh0g0-1758
Copy link
Member

Hey @RKSimon, I would like to work on this.

@changkhothuychung
Copy link
Contributor

Hi I would like to work on this if this is available

@EugeneZelenko
Copy link
Contributor

@changkhothuychung: Just create pull request and mention it on this page.

@Sh0g0-1758
Copy link
Member

Sh0g0-1758 commented Mar 11, 2024

Does this issue aim to look at both the optimization of "Average of two integers" or only alternative code targets for absolute difference in the TargetLowering.cpp file? The description of the issue also seems a little open to me. Can you please refine it with fine-grained tasks?

In the original thread, a dev mentioned this :

Can I open a new discussion regarding a better implementation of AvgFloor and AvgCeil in TargetLowering::expandABD?

I don't see why we would want to implement AvgFloor in TargetLowering::expandABD.

If I am not wrong, the optimizations that were introduced in TargetLowering::expandAddSubSat need to be introduced in TargetLowering::expandABD.

@Sh0g0-1758
Copy link
Member

During my findings, I found doz (difference or zero) to be a suitable alternative expansion. We can implement ABD(x,y) as :

doz(x,y) + doz(y,x)

Here is how it is defined :

Screenshot from 2024-03-11 15-47-23

And for the Average of two integers without using extra bits and ensuring no overflow, we can use :

(x&y)+((x^y)>>1)

@RKSimon RKSimon added invalid Resolved as invalid, i.e. not a bug and removed good first issue https://github.com/llvm/llvm-project/contribute labels Mar 11, 2024
@RKSimon
Copy link
Collaborator Author

RKSimon commented Mar 11, 2024

Closing this as it references the wrong lowering code, is too broad and rather confusing - I'll create more useful smaller issues.

@RKSimon RKSimon closed this as completed Mar 11, 2024
@Sh0g0-1758
Copy link
Member

Right, that's what I thought, was scratching my head all morning :) Nevertheless, found some interesting resources on compiler optimization.

@EugeneZelenko EugeneZelenko closed this as not planned Won't fix, can't repro, duplicate, stale Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid Resolved as invalid, i.e. not a bug llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

No branches or pull requests

5 participants