Skip to content

[MLIR][Presburger] Template Matrix to allow MPInt and Fraction #65272

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

Merged
merged 17 commits into from
Sep 18, 2023

Conversation

Abhinav271828
Copy link
Contributor

@Abhinav271828 Abhinav271828 commented Sep 4, 2023

Matrix has been templated to Matrix (for MPInt and Fraction) with explicit instantiation for both these types.
makeMatrix has been duplicated to makeIntMatrix and makeFracMatrix.

@Abhinav271828 Abhinav271828 requested a review from a team as a code owner September 4, 2023 15:44
@joker-eph joker-eph requested a review from a team September 4, 2023 18:43
@Superty Superty self-assigned this Sep 5, 2023
@Superty Superty changed the title [MLIR][Presburger] Template Matrix to allow MPInt and Fraaction [MLIR][Presburger] Template Matrix to allow MPInt and Fraction Sep 5, 2023
@github-actions github-actions bot added the mlir label Sep 5, 2023
…nstantiation

Duplicate makeMatrix to makeIntMatrix and makeFracMatrix

Implement arithmetic operations for Fraction for compatibility
…nstantiation

Duplicate makeMatrix to makeIntMatrix and makeFracMatrix

Implement arithmetic operations for Fraction for compatibility
This reverts commit 2871b0127a3cbc8d2a4cf294e731aade1ed56424.
…nstantiation

Duplicate makeMatrix to makeIntMatrix and makeFracMatrix

Implement arithmetic operations for Fraction for compatibility
…nstantiation

Duplicate makeMatrix to makeIntMatrix and makeFracMatrix

Implement arithmetic operations for Fraction for compatibility
@Abhinav271828 Abhinav271828 requested review from a team as code owners September 6, 2023 13:36
@github-actions github-actions bot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 6, 2023
@ldionne ldionne removed the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 6, 2023
@ldionne ldionne removed the request for review from a team September 6, 2023 17:45
@@ -32,7 +34,10 @@ namespace presburger {
/// (i, j) is stored at data[i*nReservedColumns + j]. The reserved but unused
/// columns always have all zero values. The reserved rows are just reserved
/// space in the underlying SmallVector's capacity.
template<typename T>
Copy link
Member

@Superty Superty Sep 18, 2023

Choose a reason for hiding this comment

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

Can you mention here that the class only works for types MPInt and Fraction as the method implementations are left in a separate .cpp file and only these two types have been explicitly instantiated there. You can drop the comment on line 39 below

@Superty Superty merged commit efca035 into llvm:main Sep 18, 2023
@NathanielMcVicar
Copy link
Contributor

It looks like this has broken the Windows MLIR bot: https://lab.llvm.org/buildbot/#/builders/13/builds/40242/steps/6/logs/stdio

FAILED: tools/mlir/lib/Analysis/Presburger/CMakeFiles/obj.MLIRPresburger.dir/Matrix.cpp.obj 
C:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\lib\Analysis\Presburger\Matrix.cpp(385): error C2220: the following warning is treated as an error
C:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\lib\Analysis\Presburger\Matrix.cpp(385): warning C4661: 'T mlir::presburger::Matrix<T>::normalizeRow(unsigned int)': no suitable definition provided for explicit template instantiation request
        with
        [
            T=mlir::presburger::Fraction
        ]
C:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\include\mlir/Analysis/Presburger/Matrix.h(165): note: see declaration of 'mlir::presburger::Matrix<mlir::presburger::Fraction>::normalizeRow'

ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
…65272)

The method implementations remain in the .cpp file; explicit instantiations have been added for these two types. 
makeMatrix has been duplicated to makeIntMatrix and makeFracMatrix.
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
…65272)

The method implementations remain in the .cpp file; explicit instantiations have been added for these two types. 
makeMatrix has been duplicated to makeIntMatrix and makeFracMatrix.
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
…65272)

The method implementations remain in the .cpp file; explicit instantiations have been added for these two types. 
makeMatrix has been duplicated to makeIntMatrix and makeFracMatrix.
zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants