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 a bug fix for LLVM as patch, applied by conan build #47

Closed
wants to merge 2 commits into from

Conversation

mhillenbrand
Copy link
Contributor

@mhillenbrand mhillenbrand commented Feb 14, 2023

For constant ops from arithmetic constants, MLIR chooses SSA value names that reflect the constant. That failed for ints with > 64 bits as the function that derived that name used a shortcut that did not apply (and assert(), as here) for ints with more than 64 bits.

We have contributed a fix to upstream LLVM in llvm/llvm-project@1ef32e7 and need to add this patch into our build of LLVM 14.0.6.

Fixes #46

@mbhealy
Copy link
Contributor

mbhealy commented Feb 14, 2023

The build seems to be failing because the patch file isn't quite matching:

llvm/14.0.6-1@qss/stable: WARN: /home/runner/work/qss-compiler/qss-compiler/.conan/data/llvm/14.0.6-1/qss/stable/source/fix-printing-larger-integer-attributes.patch: file 1/2:	 b'llvm-project/mlir/lib/Dialect/Arithmetic/IR/ArithmeticOps.cpp'
llvm/14.0.6-1@qss/stable: WARN: /home/runner/work/qss-compiler/qss-compiler/.conan/data/llvm/14.0.6-1/qss/stable/source/fix-printing-larger-integer-attributes.patch:  hunk no.1 doesn't match source file at line 95
llvm/14.0.6-1@qss/stable: WARN: /home/runner/work/qss-compiler/qss-compiler/.conan/data/llvm/14.0.6-1/qss/stable/source/fix-printing-larger-integer-attributes.patch:   expected: b'    if (intType && intType.getWidth() == 1)'
llvm/14.0.6-1@qss/stable: WARN: /home/runner/work/qss-compiler/qss-compiler/.conan/data/llvm/14.0.6-1/qss/stable/source/fix-printing-larger-integer-attributes.patch:   actual  : b''
llvm/14.0.6-1@qss/stable: /home/runner/work/qss-compiler/qss-compiler/.conan/data/llvm/14.0.6-1/qss/stable/source/fix-printing-larger-integer-attributes.patch: source file is different - b'llvm-project/mlir/lib/Dialect/Arithmetic/IR/ArithmeticOps.cpp'
ERROR: llvm/14.0.6-1@qss/stable: Error in source() method, line 100
	patch(self, patch_file=patch_file)
	ConanException: Failed to apply patch: /home/runner/work/qss-compiler/qss-compiler/.conan/data/llvm/14.0.6-1/qss/stable/source/fix-printing-larger-integer-attributes.patch
Error: Process completed with exit code 1.

@vrpascuzzi
Copy link
Contributor

Think we're still hitting the same error:

/data/llvm/14.0.6-2/_/_/source/fix-printing-larger-integer-attributes.patch: file 1/2:      b'llvm-project/mlir/lib/Dialect/Arithmetic/IR/ArithmeticOps.cpp'
llvm/14.0.6-2: WARN: /Users/vrpascuzzi/.conan/data/llvm/14.0.6-2/_/_/source/fix-printing-larger-integer-attributes.patch:  hunk no.1 doesn't match source file at line 95
llvm/14.0.6-2: WARN: /Users/vrpascuzzi/.conan/data/llvm/14.0.6-2/_/_/source/fix-printing-larger-integer-attributes.patch:   expected: b'    if (intType && intType.getWidth() == 1)'
llvm/14.0.6-2: WARN: /Users/vrpascuzzi/.conan/data/llvm/14.0.6-2/_/_/source/fix-printing-larger-integer-attributes.patch:   actual  : b''
llvm/14.0.6-2: /Users/vrpascuzzi/.conan/data/llvm/14.0.6-2/_/_/source/fix-printing-larger-integer-attributes.patch: source file is different - b'llvm-project/mlir/lib/Dialect/Arithmetic/IR/ArithmeticOps.cpp'
ERROR: llvm/14.0.6-2: Error in source() method, line 100
        patch(self, patch_file=patch_file)
        ConanException: Failed to apply patch: /Users/vrpascuzzi/.conan/data/llvm/14.0.6-2/_/_/source/fix-printing-larger-integer-attributes.patch

@mhillenbrand
Copy link
Contributor Author

Closing for now.

@mhillenbrand mhillenbrand deleted the patch-llvm branch February 27, 2023 10:16
@mhillenbrand mhillenbrand restored the patch-llvm branch February 27, 2023 10:16
@taalexander taalexander deleted the patch-llvm branch April 5, 2023 11:36
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.

Add patch from upstream LLVM to fix ssa value names for long integer constants
3 participants