Skip to content
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

Add LLVM-specific integer arithmetic operations #788

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

phate
Copy link
Owner

@phate phate commented Feb 4, 2025

This PR replaces the old bitstring operations with new LLVM specific integer arithmetic operations. The implementations have currently still the following limitations:

  1. The operations still use the bistring type. The type will potentially later replaced by an LLVM specific type.
  2. No operation specific reductions have been implemented yet. This will also happen in a follow-up PR.

At this point, the implementation only tries to get on par with the respective bitstring operations (minus the reductions). More LLVM specific semantics are going to be introduced in a future PR.

@phate
Copy link
Owner Author

phate commented Feb 4, 2025

There is still quite a bit repetition in the code, but I would like you to look over that for now. We can refactor that similar to the bitstring operations once the dust has settled.

@phate phate requested review from haved and caleridas February 4, 2025 20:05
@phate phate force-pushed the Integer-ArithmeticReplacement branch from 3d28242 to 65faa72 Compare February 5, 2025 05:58
Copy link
Collaborator

@haved haved left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interface looks nice, with some duplication of course.

I have one question about the use of result(0).

In order to reduce duplication for some of the methods, maybe use of an additional helper superclass based on a curiously recurring template could be nice?

jlm/llvm/ir/operators/IntegerOperations.cpp Outdated Show resolved Hide resolved
@phate
Copy link
Owner Author

phate commented Feb 6, 2025

@haved I would not like to reduce the duplication yet, before we have not modeled all the semantics of the corresponding LLVM instructions. Once we have all the semantics in there, we can start to simplify. I hope that is okay for you.

@phate phate requested a review from haved February 6, 2025 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants