Add LLVM 14 support #830

apmasell · 2022-04-05T19:24:16Z

No description provided.

stuartarchibald · 2022-04-06T15:59:49Z

Thanks for starting work on LLVM 14 support. In thinking about how to make it easier to review the changes needed, perhaps the following approach might work:

Creating a separate patch for the C++ code formatting changes, this gets merged first so as to minimise future diffs and to make it easy to spot code changes that are real vs. formatting.
Subsequently adding support for LLVM 14 including build and patch updates as a separate PR, then merging this next.
Finally adding support for the addition LLVM pass bindings/wrappers and merging this last.

What do you think?

apmasell · 2022-04-06T19:12:49Z

Sure. I've created #831 for the code formatting changes. I'm going to leave this PR open so I can use it for the build farm.

stuartarchibald · 2022-04-08T08:14:54Z

Sure. I've created #831 for the code formatting changes. I'm going to leave this PR open so I can use it for the build farm.

Thanks, keeping this open for testing purposes sounds like a good plan.

Hardcode84 · 2022-07-24T23:17:36Z

Hi,

Here is svml patch for llvm 14 Hardcode84/llvm-project@bc2dcd1

The newset version can depend on llvm:13 which is still in ::gentoo And there is ongoing efforts of migrating to llvm:14 See numba/llvmlite#830 Signed-off-by: Yiyang Wu <xgreenlandforwyy@gmail.com>

The newset version can depend on llvm:13 which is still in ::gentoo And there is ongoing efforts of migrating to llvm:14 See numba/llvmlite#830 Signed-off-by: Yiyang Wu <xgreenlandforwyy@gmail.com> Closes: #1164 Signed-off-by: Andrew Ammerlaan <andrewammerlaan@gentoo.org>

sklam · 2022-09-20T18:17:19Z

conda-recipes/llvmdev/meta.yaml

-{% set sha256_lld = "017a788cbe1ecc4a949abf10755870519086d058a2e99f438829aef24f0c66ce" %}
+{% set shortversion = "14.0" %}
+{% set version = "14.0.1" %}
+{% set sha256_llvm = "1a3c2e57916c5a70153aaf0a0e6f1230d6368b9e0f4d04dcb9e039a31b1cd4e6" %}
 {% set build_number = "5" %}


rmbr to reset this before merge

sklam · 2022-09-20T18:17:38Z

conda-recipes/llvmlite/meta.yaml

+    - llvmdev 14.0.1 *5 # [(osx and arm64)]
+    - llvmdev 14.0.1 *4 # [not ((osx and arm64) or win)]
+    - llvmdev 14.0.1 4 # [win]


Suggested change

- llvmdev 14.0.1 *5 # [(osx and arm64)]

- llvmdev 14.0.1 *4 # [not ((osx and arm64) or win)]

- llvmdev 14.0.1 4 # [win]

- llvmdev 14.0.1

detrout · 2022-10-25T17:19:22Z

Thanks for pointing me here from the llvmlite patch.

How important are the patches to llvm itself? There seems to be a bunch of changes in the conda recipes, are those needed outside of a conda environment?

apmasell · 2022-10-25T19:16:55Z

The patches to LLVM 14 that are included with the recipe are pretty important. Numba uses SVML and that is not in LLVM mainline and needs to be patched in. Some of the other changes are for building on different environments (MacOS and Windows can be uncooperative).

apmasell · 2022-11-07T12:00:08Z

That's not on our roadmap yet. LLVM 15 has a number of breaking changes in the way it handles pointers, and we aren't prepared to handle those yet.

sklam

@apmasell, try vs2019 and pinning to 14.2 toolset:

conda-recipes/llvmdev/bld.bat

gmarkall · 2022-11-29T11:43:49Z

/azp run

azure-pipelines · 2022-11-29T11:43:58Z

Azure Pipelines successfully started running 1 pipeline(s).

sklam

Before proceeding, we should get more cases. Particular, we can to see if there are other patterns if multiple exceptions of different exception type are raised.

e.g.

if cond_a:
    raise ValueError
elif cond_b:
    raise TypeError
else:
    return ok

sklam · 2022-12-09T15:58:35Z

ffi/custom_passes.cpp

+            auto operand = first->getOperand(0);
+            // If the operand is a constant, check if it indicates an exception
+            auto int_operand = dyn_cast<ConstantInt>(operand);
+            if (int_operand && !int_operand->isZeroValue()) {


Seems like it will always be one according to the old code.

// and then cast the const as Metadata (Numba sets this as literal // 1) auto data = dyn_cast<ConstantAsMetadata>(operand.get());

But other negative non-zero value can indicate other non-raising condition.

See https://github.com/numba/numba/blob/9d4f82df7f0426d89b189eafc7acdc6bf719c716/numba/core/callconv.py#L473-L479. It's Numba handling the return code. For this refprune case, we used to only care about user exception. The existing Numba code seems to only ever emit 1. Numba callconv may need to clean up.

The case that is failing in the Numba repository emits a 1, so I'm not sure what's going on with the calling convetions.

sklam · 2022-12-09T16:01:43Z

ffi/custom_passes.cpp

+            if (phi_operand) {
+                auto arg_value = dyn_cast<ConstantInt>(
+                    phi_operand->getIncomingValueForBlock(bb));
+                return arg_value && !arg_value->isZeroValue();


same as earlier, not all non-zero value means exception

The way the LLVM constants are set up, I can only check for 0, 1, and -1. https://llvm.org/doxygen/classllvm_1_1Constant.html

sklam · 2022-12-09T16:05:00Z

ffi/custom_passes.cpp

+            auto phi_operand = dyn_cast<PHINode>(operand);
+            if (phi_operand) {
+                auto arg_value = dyn_cast<ConstantInt>(
+                    phi_operand->getIncomingValueForBlock(bb));


Look at PHI value for the return code is clever. That may save us needing the metadata all together.

It is restricted to the cases where the PHI values contain constants. It would be a lot more effort to untangle if they were other results.

gmarkall · 2023-01-20T10:24:39Z

/azp run

azure-pipelines · 2023-01-20T10:24:50Z

Azure Pipelines successfully started running 1 pipeline(s).

gmarkall · 2023-01-20T10:25:26Z

CI was messed up because of the recent issue with it, so I'm just running the pipelines because I'm curious what the tests look like right now.

sklam

Changes needed:

The build recipe needs to be changed to build llvm11 for linux-aarch64 per Packaging issue caused due to llvm14 RuntimeDyLd issue on linux-aarch64 numba#8756
Code changes in the tests are loosing the original intention. I've made suggestions to retain the original intention.

sklam · 2023-02-15T21:59:19Z

llvmlite/tests/test_refprune.py

@@ -371,7 +371,7 @@ def test_fanout_1(self):
        self.assertEqual(stats.fanout, 3)

    fanout_2 = r"""
-define void @main(i8* %ptr, i1 %cond) {
+define void @main(i8* %ptr, i1 %cond, i8** %excinfo) {


Is this needed? Fanout test shouldn't depend on excinfo.

llvmlite/tests/test_refprune.py

sklam · 2023-02-15T22:03:39Z

llvmlite/tests/test_refprune.py

+    ret i32 1    ; BAD; all tails are raising without decref
 bb_C:
-    ret i32 1, !ret_is_raise !0    ; BAD; all tails are raising without decref
+    ret i32 1    ; BAD; all tails are raising without decref


The original intention of the test is to verify that no pruning is done when all branches are raising. I'd suggest adding the store to excinfo for these branches so isRaising will return True.

sklam · 2023-02-15T22:09:44Z

conda-recipes/llvmdev/meta.yaml

-{% set sha256_llvm = "ce8508e318a01a63d4e8b3090ab2ded3c598a50258cc49e2625b9120d4c03ea5" %}
-{% set sha256_lld = "017a788cbe1ecc4a949abf10755870519086d058a2e99f438829aef24f0c66ce" %}
-{% set build_number = "5" %}
+{% set shortversion = "14.0" %}


We'll need split build for linux-aarch64 to use llvm11 as discussed in numba/numba#8756.

See
https://github.com/numba/llvmlite/pull/599/files#diff-a97126465bc884942a676793bbd8f9443dc410f5aa05c57d9bbf1d0ce5a6c437R2-R18 for reference.

stuartarchibald · 2023-02-23T17:34:15Z

NOTE: need to check if any documentation needs updating.

stuartarchibald · 2023-02-23T17:36:15Z

conda-recipes/llvmdev/build.sh

-# run the tests, skip some on linux-32
-cd ../test
-if [[ $ARCH == 'i686' ]]; then
-    ../build/bin/llvm-lit -vv Transforms Analysis CodeGen/X86
-else
-    ../build/bin/llvm-lit -vv Transforms ExecutionEngine Analysis CodeGen/X86
-fi


What is the reason for removing these tests? If it's because they are in build.sh perhaps they should be moved to run_test.sh?

We are having trouble getting any test to work. Deferring adding the test back to another PR given that this PR has passed buildfarm multiple times.

Please could a new issue be opened to track this so it's visible as to what is failing/has been tried already. I also think it is ok on this occasion to defer this work to another patch given how many runs through the build farm this has had. Many thanks.

CC @apmasell.

As titled.

llvmlite/tests/test_refprune.py

sklam · 2023-03-07T17:57:43Z

llvmlite/tests/test_refprune.py

+        # Change in behaviour: ignore bad metadata
+        self.assertEqual(stats.fanout_raise, 2)


The original intention is to check that pruning does not occur when there's a bad metadata; e.g. misspelled metadata name. See suggestion above.

llvmlite/tests/test_refprune.py

Co-authored-by: Siu Kwan Lam <1929845+sklam@users.noreply.github.com>

llvmlite/tests/test_refprune.py

Co-authored-by: Siu Kwan Lam <1929845+sklam@users.noreply.github.com>

stuartarchibald · 2023-03-09T11:18:11Z

The installation instructions need updating from here onward:

llvmlite/docs/source/admin-guide/install.rst

Line 178 in dffe582

Compiling LLVM

suggest deferring this to a subsequent PR?

sklam · 2023-03-09T14:50:20Z

ffi/Makefile.osx

-CXX = clang++ -std=c++11 -stdlib=libc++
-CXXFLAGS = $(LLVM_CXXFLAGS)
+CXX = clang++
+CXXFLAGS = $(LLVM_CXXFLAGS) -O3


@apmasell, is the -O3 needed?

i am removing it in my PR

sklam

Approving for merge.
Installation docs defer to separate PR

apmasell added the 2 - In Progress label Apr 5, 2022

stuartarchibald added this to the 0.39.0 RC milestone Apr 8, 2022

sklam modified the milestones: 0.39.0 RC, PR Backlog Jun 1, 2022

h-vetinari mentioned this pull request Aug 25, 2022

Reactivate SVML patches conda-forge/llvmdev-feedstock#123

Open

sklam reviewed Sep 20, 2022

View reviewed changes

apmasell mentioned this pull request Oct 24, 2022

Enable LLVM-12 #802

Closed

sklam reviewed Nov 8, 2022

View reviewed changes

conda-recipes/llvmdev/bld.bat Outdated Show resolved Hide resolved

conda-recipes/llvmdev/bld.bat Outdated Show resolved Hide resolved

h-vetinari mentioned this pull request Nov 25, 2022

add SVML patch for LLVM 14 conda-forge/llvmdev-feedstock#192

Merged

apmasell mentioned this pull request Dec 1, 2022

SVML fails to produce Haswell-friendly code with LLVM 14 patch #894

Closed

This was referenced Dec 6, 2022

{lang}[foss/2022a] numba v0.56.4 w/ Python 3.10.4 + CUDA 11.7.0 easybuilders/easybuild-easyconfigs#16764

Closed

{lang}[foss/2022a] numba v0.56.4 w/ Python 3.10.4 + CUDA 11.7.0 easybuilders/easybuild-easyconfigs#16814

Merged

sklam reviewed Dec 9, 2022

View reviewed changes

MehdiChinoune mentioned this pull request Dec 22, 2022

Support LLVM 15 #897

Closed

sklam requested changes Feb 15, 2023

View reviewed changes

sklam added 4 - Waiting on author and removed 3 - Ready for Review labels Feb 15, 2023

stuartarchibald modified the milestones: PR Backlog, v0.40.0 Feb 23, 2023

stuartarchibald reviewed Feb 23, 2023

View reviewed changes

Remove unused metadata

9523fad

As titled.

sklam self-requested a review March 7, 2023 16:02

sklam added 4 - Waiting on reviewer and removed 4 - Waiting on author labels Mar 7, 2023

sklam reviewed Mar 7, 2023

View reviewed changes

apmasell and others added 2 commits March 7, 2023 15:02

Update llvmlite/tests/test_refprune.py

51ac097

Co-authored-by: Siu Kwan Lam <1929845+sklam@users.noreply.github.com>

Update llvmlite/tests/test_refprune.py

37a1843

Co-authored-by: Siu Kwan Lam <1929845+sklam@users.noreply.github.com>

sklam reviewed Mar 7, 2023

View reviewed changes

llvmlite/tests/test_refprune.py Outdated Show resolved Hide resolved

Update llvmlite/tests/test_refprune.py

cae23af

Co-authored-by: Siu Kwan Lam <1929845+sklam@users.noreply.github.com>

sklam approved these changes Mar 7, 2023

View reviewed changes

sklam added Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm and removed 4 - Waiting on reviewer labels Mar 7, 2023

sklam mentioned this pull request Mar 8, 2023

Aarch64 split build for LLVM14 #910

Merged

apmasell mentioned this pull request Mar 8, 2023

LLVM14 test suite not run on llvmdev pacakage build #913

Open

sklam reviewed Mar 9, 2023

View reviewed changes

sklam approved these changes Mar 9, 2023

View reviewed changes

sklam added BuildFarm Passed For PRs that have been through the buildfarm and passed 5 - Ready to merge and removed Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm labels Mar 9, 2023

sklam merged commit a78869d into numba:main Mar 9, 2023

gmarkall mentioned this pull request Mar 9, 2023

Support for LLVM 15.0.7 #914

Open

apmasell deleted the llvm_14_support branch March 29, 2023 18:30

		# Change in behaviour: ignore bad metadata
		self.assertEqual(stats.fanout_raise, 2)

Add LLVM 14 support #830

Add LLVM 14 support #830

Conversation

apmasell commented Apr 5, 2022

stuartarchibald commented Apr 6, 2022

apmasell commented Apr 6, 2022

stuartarchibald commented Apr 8, 2022

Hardcode84 commented Jul 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

detrout commented Oct 25, 2022

apmasell commented Oct 25, 2022

apmasell commented Nov 7, 2022

sklam left a comment

Choose a reason for hiding this comment

gmarkall commented Nov 29, 2022

azure-pipelines bot commented Nov 29, 2022

sklam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmarkall commented Jan 20, 2023

azure-pipelines bot commented Jan 20, 2023

gmarkall commented Jan 20, 2023

sklam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuartarchibald commented Feb 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuartarchibald commented Mar 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sklam left a comment

Choose a reason for hiding this comment