Pr/ci add icx #1473

AlexMWells · 2022-03-05T01:59:01Z

Description

Add compiler support for Intel(r) oneAPI DPC++/C++ Compiler (based on LLVM technology) aka icx.

Detect if CMAKE_CXX_COMPILER_ID matches "IntelLLVM" and set CMAKE_COMPILER_IS_INTELCLANG=1
NOTE: CMAKE_COMPILER_IS_INTEL will not be set, that is reserved for the classic Intel(r) C++ Compiler.

Define OSL_INTEL_LLVM_COMPILER to hold an encoded Intel(r) LLVM Compiler version (e.g. 20220000), or 0 if not an Intel(r) LLVM Compiler.
Define OSL_INTEL_CLANG_VERSION to hold the encoded Clang version the Intel(r) LLVM Compiler is based on (e.g. 140000),
or 0 if not an Intel(r) LLVM compiler.

Ensure OSL_CLANG_VERSION is when its a vanilla clang compiler, not apple or Intel(r) oneAPI DPC++/C++ Compiler.

Added OSL_ANY_CLANG which is set for any clang based compiler.

Removed OSL_NON_CLANG_COMPILER.
Updated uses of OSL_NON_CLANG_COMPILER with more specific and understandable combinations of OSL_ANY_CLANG, OSL_INTEL_CLANG_VERSION, and OSL_INTEL_COMPILER.

Make sure our math calculations are consistent, especially division. Different compilers may have choose to use reciprocal division loosing precision causing slightly different results which can lead to aliasing differences when running the testsuite.
Disabled reciprocal division for all projects.

if -ffast-math is enabled to to the underlying compiler, inf's and NaN's may not be handled properly (by design for preformance). When building liboslexec, ensure any source files that require proper INF or NaN handling have additional command line flags passed if needed (as the default behavior maybe -ffast-math):
shadingsys.cpp
wide/wide_shadingsys
wide/wide_optest_float

In in impl_transform_normal_masked, enable unitialized Matrix44 because all of its members get overwritten anyway,

Tests

Added icx-vp2022 to CI plan to exercise icx.

Added alternative test results where needed, using '.icx.' as shorthand for the Intel(r) oneAPI DPC++/C++ Compiler in alternative test result filenames.
Changed some tests to use the pretty(value) which clamps values close to 0 to be 0 to avoid lots of different alternative rest result files when using printf.
Added "--center" option to several regression tests to avoid aliasing in results between SIMD batched and scalar execution.

Checklist:

I have read the contribution guidelines.
I have previously submitted a Contributor License Agreement.
I have updated the documentation, if applicable.
I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
My code follows the prevailing code style of this project.

…LLVM technology). Detect if CMAKE_CXX_COMPILER_ID matches "IntelLLVM" and set CMAKE_COMPILER_IS_INTELCLANG=1 NOTE: CMAKE_COMPILER_IS_INTEL will not be set, that is reserved for the classic Intel(r) C++ Compiler. Define OSL_INTEL_LLVM_COMPILER to hold an encoded Intel(r) LLVM Compiler version (e.g. 20220000), or 0 if not an Intel(r) LLVM Compiler. Define OSL_INTEL_CLANG_VERSION to hold the encoded Clang version the Intel(r) LLVM Compiler is based on (e.g. 140000), or 0 if not an Intel(r) LLVM compiler. Ensure OSL_CLANG_VERSION is when its a vanilla clang compiler, not apple or Intel(r) oneAPI DPC++/C++ Compiler. Added OSL_ANY_CLANG which is set for any clang based compiler. Removed OSL_NON_CLANG_COMPILER. Updated uses of OSL_NON_CLANG_COMPILER with more specific and understandable combinations of OSL_ANY_CLANG, OSL_INTEL_CLANG_VERSION, and OSL_INTEL_COMPILER. Make sure our math calculations are consistent, especially division. Different compilers may have choose to use reciprocal division loosing precision causing slightly different results which can lead to aliasing differences when running the testsuite Disabled reciprocal division for all projects. if -ffast-math is enabled to to the underlying compiler, inf's and NaN's may not be handled properly (by design) When building liboslexec, ensure any source files that require proper INF or NaN handling have additional command line flags passed if needed (as the default behavior maybe -ffast-math): shadingsys.cpp wide/wide_shadingsys wide/wide_optest_float Enable unitialized Matrix44, because all of its members get overwritten anyway, in impl_transform_normal_masked. Added alternative test results where needed, wsing '.icx.' as shorthand for the Intel(r) oneAPI DPC++/C++ Compiler in alternative test result filenames. Changed some tests to use the pretty(value) which clamps values close to 0 to be 0 to avoid lots of different alternative rest result files when using printf. Added "--center" option to several regression tests to avoid aliasing in results between SIMD batched and scalar execution. Signed-off-by: Alex M. Wells <[email protected]>

Signed-off-by: Alex M. Wells <[email protected]>

lgritz

Great!

lgritz · 2022-03-05T02:01:46Z

CMakeLists.txt

@@ -175,6 +175,21 @@ include_directories (
  )


+# Make sure our math calculations are consistent,
+# especially division.  Different compilers may have choose to use
+# reciprocal division loosing precision causing slightly different


Suggested change

# reciprocal division loosing precision causing slightly different

# reciprocal division losing precision causing slightly different

lgritz · 2022-03-05T02:03:07Z

CMakeLists.txt

+        add_compile_options("-prec-div")
+    endif ()
+elseif (CMAKE_COMPILER_IS_CLANG OR CMAKE_COMPILER_IS_APPLECLANG OR CMAKE_COMPILER_IS_INTELCLANG)
+    add_compile_options("-fno-reciprocal-math")


Does this have performance implications we should be aware of? Is this something we should do all the time, or just for CI tests for the sake of uniformity of results across compilers?

Yes it has performance implications (as a reciprocal divide instruction combined with a multiply is faster).
Yes its just for sake of CI tests and the sake of uniformity of results across compilers!
There is a difference in default behavior among compilers, and hardware availability for the reciprocal divide.

For performance, I would do the reverse, explicitly enable reciprocal divides. However precision will be lost, perhaps more than a renderer would want.

As a programmer, I feel that the programmer probably should of written (1/divisor)*number instead of number/divisor if their algorithm would be OK with the loss in precision.

Also tests themselves could be updated to be more resilient (example is using --center) and alternate results could be added as well. FMA's have similar issue but actually increase precision/correctness of results vs. a reciprocal divide which looses precision.

lgritz · 2022-03-06T01:55:18Z

.github/workflows/ci.yml

+      FMT_VERSION: 7.1.3
+      OPENIMAGEIO_VERSION: master
+      OPENIMAGEIO_CMAKE_FLAGS: -DBUILD_FMT_VERSION=7.1.3


I believe the FMT_VERSION and OPENIMAGEIO_CMAKE_FLAGS lines are not necessary for icx, only icc.

good catch, I'll try removing those.

lgritz · 2022-03-06T01:56:59Z

.github/workflows/ci.yml

+      PYTHON_VERSION: 3.9
+      USE_BATCHED: b8_AVX2_noFMA
+      # USE_BATCHED: b16_AVX512
+      # USE_SIMD: avx2,f16c


I had commented out the USE_SIMD for icc because it uses a different syntax than the -m arguments that this gets turned into by compiler.cmake. But icx accepts the same -m arguments as clang. So you can re-enable that line for icx.

I'll try adding USE_SIMD: avx2,f16c back in

It worked for me on the OIIO side.

I've been contemplating a change to the logic in compiler.cmake to basically be sure to do the right thing to the USE_SIMD for icc under the covers (i.e., if it sees USE_SIMD=avx512f, instead of just passing on -mavx512f, which I think is not recognized by icc, it would translates to -axCORE-AVX512 [I think]). But now that I see that icx recognizes all the same commands as clang, I'm wondering if extra work for icc is really worth the trouble (or if users should just use EXTRA_CPP_ARGS to set the right thing if they know it) if icc users are going to tend to switch to icx over time anyway.

refs:
https://www.intel.com/content/www/us/en/develop/documentation/cpp-compiler-developer-guide-and-reference/top/compiler-reference/compiler-options/compiler-option-details/code-generation-options/m.html
https://www.intel.com/content/www/us/en/develop/documentation/cpp-compiler-developer-guide-and-reference/top/compiler-reference/compiler-options/compiler-option-details/code-generation-options/x-qx.html

Not a make file guru, if passing native flags, then just make user pass them in the 1st place. Or have those be OSL defined and controlled flags that get translated. Technically adding "-m" does count as translating, so having it spit out "-axCORE-AVX512" seems totally reasonable. You do know the "-ax" is automatic dispatch that generates multiple versions of functions with a spring board to jump to the correct one based on CPUID?

lgritz · 2022-03-06T01:56:59Z

.github/workflows/ci.yml

+      PYTHON_VERSION: 3.9
+      USE_BATCHED: b8_AVX2_noFMA
+      # USE_BATCHED: b16_AVX512
+      # USE_SIMD: avx2,f16c


I had commented out the USE_SIMD for icc because it uses a different syntax than the -m arguments that this gets turned into by compiler.cmake. But icx accepts the same -m arguments as clang. So you can re-enable that line for icx.

lgritz · 2022-03-06T01:58:57Z

src/build-scripts/gh-installdeps.bash

+        if [[ "$CXX" == "icpc" || "$CC" == "icc" || "$USE_ICC" != "" ]] ; then
+            icpc --version
+        fi
+        if [[ "$CXX" == "icpx" || "$CC" == "icx" || "$USE_ICX" != "" ]] ; then
+            icpx --version
+        fi


I think these lines (printing the versions) are wholly unnecessary, since early in the "build" stage, the cmake scripts print the compiler and version, so it's already easy to verify looking at the logs. (In fact, these lines can be misleading, since it only tells you that an icpc or icpx exist, but not what we actually are building with it!)

Up to you, we can remove them entirely or add more context

Suggested change

if [[ "$CXX" == "icpc" || "$CC" == "icc" || "$USE_ICC" != "" ]] ; then

icpc --version

fi

if [[ "$CXX" == "icpx" || "$CC" == "icx" || "$USE_ICX" != "" ]] ; then

icpx --version

fi

if [[ "$CXX" == "icpc" || "$CC" == "icc" || "$USE_ICC" != "" ]] ; then

echo Verify installation of Intel(r) C++ Compiler

icpc --version

fi

if [[ "$CXX" == "icpx" || "$CC" == "icx" || "$USE_ICX" != "" ]] ; then

echo Verify installation of Intel(r) oneAPI DPC++/C++ Compiler

icpx --version

fi

It prints these things in the "build setup" part of the log, which I guess is ok.

I put this "echo" in early in the process of my first icc patch, where I was just trying to debug the installation of the icc package itself (didn't even get as far as the osl build), so this print made sense. But now that it's working, I'm not sure I see a point in it. Though it's not harmful, either.

Frequently, when there are build problems, the part I open up and inspect is the "build" section, where at cmake startup it already prints which compiler and version it's using (so this is entirely redundant), and in the same area it prints the versions it finds of all the library dependencies.

lgritz · 2022-03-06T01:58:57Z

src/build-scripts/gh-installdeps.bash

+        if [[ "$CXX" == "icpc" || "$CC" == "icc" || "$USE_ICC" != "" ]] ; then
+            icpc --version
+        fi
+        if [[ "$CXX" == "icpx" || "$CC" == "icx" || "$USE_ICX" != "" ]] ; then
+            icpx --version
+        fi


I think these lines (printing the versions) are wholly unnecessary, since early in the "build" stage, the cmake scripts print the compiler and version, so it's already easy to verify looking at the logs. (In fact, these lines can be misleading, since it only tells you that an icpc or icpx exist, but not what we actually are building with it!)

lgritz · 2022-03-06T01:59:14Z

src/build-scripts/gh-installdeps.bash

+        if [[ "$CXX" == "icpc" || "$CC" == "icc" || "$USE_ICC" != "" ]] ; then
+            icpc --version
+        fi
+        if [[ "$CXX" == "icpx" || "$CC" == "icx" || "$USE_ICX" != "" ]] ; then
+            icpx --version
+        fi


Unnecessary -- see other note

lgritz · 2022-03-06T01:59:14Z

src/build-scripts/gh-installdeps.bash

+        if [[ "$CXX" == "icpc" || "$CC" == "icc" || "$USE_ICC" != "" ]] ; then
+            icpc --version
+        fi
+        if [[ "$CXX" == "icpx" || "$CC" == "icx" || "$USE_ICX" != "" ]] ; then
+            icpx --version
+        fi


Unnecessary -- see other note

lgritz · 2022-03-06T02:02:47Z

src/include/OSL/platform.h

+#  define OSL_INTEL_LLVM_COMPILER __INTEL_LLVM_COMPILER
+#  define OSL_INTEL_CLANG_VERSION (10000*__clang_major__ + 100*__clang_minor__ + __clang_patchlevel__)


Are both of these necessary? For the other compilers, we only define OSL_COMPILERNAME_VERSION, set to some encoding of the version, and to 0 if that is not the compiler being used. If #if OSL_INTEL_LLVM_COMPILER isn't every going to evaluate differently than #if OSL_INTEL_CLANG_VERSION, then let's just eliminate the former one. (I don't have a preference for whether you call it OSL_INTEL_CLANG_VERSION or OSL_INTEL_LLVM_VERSION... pick whichever you think is better, and I'll change the OIIO side to conform.)

Depends on what you want to check on. If you have a workaround that is more clang version based, then it might be nice to check the clang version, instead of having to figure out which __INTEL_LLVM_COMPILER version's map to which clang versions (20220000 __INTEL_LLVM_COMPILER maps to clang version 140000. If only 1, then OSL_INTEL_LLVM_COMPILER as that might rev several times without changing the underlying clang compiler.

Oh, I see what you mean. Are you saying that OSL_INTEL_LLVM_COMPILER holds the actual Intel version number, while OSL_INTEL_CLANG_VERSION holds the version of the clang that it's emulating or derived from?

I misunderstood, and thought you were using OSL_INTEL_LLVM_COMPILER simply as a bool of sorts.

So, in that case, let me change my recommendation. Let's change OSL_INTEL_LLVM_COMPILER to OSL_INTEL_LLVM_COMPILER_VERSION to make clear that it's a version number, not just a bool, and thus match OSL_GNUC_VERSION, OSL_APPLE_CLANG_VERSION, etc.

Seems valid reasoning. This means the older
OSL_INTEL_COMPILER
which also has the compiler version (its not just a bool), should be renamed as well. And you were already suggesting
OSL_INTEL_CLASSIC_COMPILER, so should it be
OSL_INTEL_CLASSIC_COMPILER_VERSION
?

Yeah, I think that makes it maximally self-documenting and symmetric to how we did the other compilers.

lgritz · 2022-03-06T02:02:47Z

src/include/OSL/platform.h

+#  define OSL_INTEL_LLVM_COMPILER __INTEL_LLVM_COMPILER
+#  define OSL_INTEL_CLANG_VERSION (10000*__clang_major__ + 100*__clang_minor__ + __clang_patchlevel__)


Are both of these necessary? For the other compilers, we only define OSL_COMPILERNAME_VERSION, set to some encoding of the version, and to 0 if that is not the compiler being used. If #if OSL_INTEL_LLVM_COMPILER isn't every going to evaluate differently than #if OSL_INTEL_CLANG_VERSION, then let's just eliminate the former one. (I don't have a preference for whether you call it OSL_INTEL_CLANG_VERSION or OSL_INTEL_LLVM_VERSION... pick whichever you think is better, and I'll change the OIIO side to conform.)

lgritz · 2022-03-06T02:05:32Z

src/include/OSL/wide.h

@@ -3139,7 +3139,7 @@ template<typename DataT, int WidthT>
 OSL_FORCEINLINE bool
 testIfAnyLaneIsNonZero(const Wide<DataT, WidthT>& wvalues)
 {
-#if OSL_NON_INTEL_CLANG
+#if OSL_ANY_CLANG && !OSL_INTEL_COMPILER && !OSL_INTEL_LLVM_COMPILER


Aside: should we consider renaming OSL_INTEL_COMPILER to OSL_INTEL_CLASSIC_COMPILER just to clarify that it only means icc, and not "any of the Intel compilers"? (I found myself trying to re-remind myself that constantly as I read this code and worked on the OIIO side.)

For OSL, I would be fine (prefer) OSL_INTEL_CLASSIC_COMPILER, but do we keep OSL_INTEL_COMPILER around (comment it deprecated)?

On one hand, this is a public header, so that would argue for keeping the old one as a (deprecated) alias until we're moving to a 2.0 release where we're allowed to break user source code. On the other hand, this preprocessor symbol is for us and it's not a documented part of the public APIs?

Your call. I don't feel strongly either way in this particular case.

Although a compilation error would be annoying, If downstream project is using OSL_INTEL_COMPILER, they should get a nice compile error and easily update it on their end. So, I'll just drop it.

lgritz · 2022-03-06T02:05:32Z

src/include/OSL/wide.h

@@ -3139,7 +3139,7 @@ template<typename DataT, int WidthT>
 OSL_FORCEINLINE bool
 testIfAnyLaneIsNonZero(const Wide<DataT, WidthT>& wvalues)
 {
-#if OSL_NON_INTEL_CLANG
+#if OSL_ANY_CLANG && !OSL_INTEL_COMPILER && !OSL_INTEL_LLVM_COMPILER


Aside: should we consider renaming OSL_INTEL_COMPILER to OSL_INTEL_CLASSIC_COMPILER just to clarify that it only means icc, and not "any of the Intel compilers"? (I found myself trying to re-remind myself that constantly as I read this code and worked on the OIIO side.)

lgritz · 2022-03-06T02:19:40Z

src/liboslnoise/wide/wide_gabornoise.h

@@ -41,7 +41,8 @@ wide_gabor(Masked<Dual2<float>> wResult, Wide<const Dual2<float>> wX,
        sfm::GaborUniformParams gup(*opt);
        __OSL_SETUP_WIDE_DIRECTION

-#if !OSL_NON_INTEL_CLANG  // Control flow too complex for clang's loop vectorizor
+#if (!OSL_ANY_CLANG || OSL_INTEL_COMPILER)


Aside, that we don't need to address in this PR:

There are a lot of instances of this construct that means "if we trust omp simd for complex loops". If a future clang is better with these, we will need to not only change this in quite a few locations, but also it'll need to be a more complicated test that checks which specific version of which clang compilers.

Perhaps it is better to JUST ONCE, in platform.h, have a single

#if OSL_OPENMP_SIMD && (OSL_GCC_VERSION || OSL_INTEL_COMPILER || OSL_INTEL_CLANG_VERSION || ...etc...) #define OSL_OMP_SIMD_COMPLEX_LOOP OSL_PRAGMA(omp simd simdlen(__OSL_WIDTH)) #else #define OSL_OMP_SIMD_COMPLEX_LOOP #endif

and then all these spots can simply say

OSL_OMP_SIMD_COMPLEX_LOOP for (...)

So in a future where certain particular versions of clang are known to handle these loops well, then we can modify the single master definition and have all the loops do the right thing with no change of cut-and-paste errors or missing any.

The OSL_NON_INTEL_CLANG always bugged me when reading it, and it got worse with the new mix of classic and llvm based compilers. So I would just go ahead and make this change now and feel better about it!

I still like to pass arguments vs. assuming simdlen(__OSL_WIDTH), but perhaps something like

#define OSL_OMP_SIMD_LOOP(...) OSL_OMP_PRAGMA(omp simd __VA_ARGS__) #if (OSL_GCC_VERSION || OSL_INTEL_COMPILER) #define OSL_OMP_COMPLEX_SIMD_LOOP(...) OSL_OMP_PRAGMA(omp simd __VA_ARGS__) #else // Ignore requests to vectorize complex SIMD loops for certain compiler's/versions #define OSL_OMP_COMPLEX_SIMD_LOOP(...) #endif

I think that's reasonable, yes.

lgritz · 2022-03-06T02:19:40Z

src/liboslnoise/wide/wide_gabornoise.h

@@ -41,7 +41,8 @@ wide_gabor(Masked<Dual2<float>> wResult, Wide<const Dual2<float>> wX,
        sfm::GaborUniformParams gup(*opt);
        __OSL_SETUP_WIDE_DIRECTION

-#if !OSL_NON_INTEL_CLANG  // Control flow too complex for clang's loop vectorizor
+#if (!OSL_ANY_CLANG || OSL_INTEL_COMPILER)


Aside, that we don't need to address in this PR:

There are a lot of instances of this construct that means "if we trust omp simd for complex loops". If a future clang is better with these, we will need to not only change this in quite a few locations, but also it'll need to be a more complicated test that checks which specific version of which clang compilers.

Perhaps it is better to JUST ONCE, in platform.h, have a single

#if OSL_OPENMP_SIMD && (OSL_GCC_VERSION || OSL_INTEL_COMPILER || OSL_INTEL_CLANG_VERSION || ...etc...) #define OSL_OMP_SIMD_COMPLEX_LOOP OSL_PRAGMA(omp simd simdlen(__OSL_WIDTH)) #else #define OSL_OMP_SIMD_COMPLEX_LOOP #endif

and then all these spots can simply say

OSL_OMP_SIMD_COMPLEX_LOOP for (...)

So in a future where certain particular versions of clang are known to handle these loops well, then we can modify the single master definition and have all the loops do the right thing with no change of cut-and-paste errors or missing any.

lgritz · 2022-03-06T02:27:30Z

If you would like to test your osl-side build CI against my in-review OIIO-side changes, you can add these lines to the workflows/ci.yml in the env section of the icx test (and remove it before we're ready for the final merge):

  OPENIMAGEIO_REPO: lgritz/oiio
  OPENIMAGEIO_VERSION: lg-icx

That will cause the "build_openimageio.bash" script used by the CI setup to pull MY branch instead of the master of the official repo.

lgritz · 2022-03-06T02:27:30Z

If you would like to test your osl-side build CI against my in-review OIIO-side changes, you can add these lines to the workflows/ci.yml in the env section of the icx test (and remove it before we're ready for the final merge):

  OPENIMAGEIO_REPO: lgritz/oiio
  OPENIMAGEIO_VERSION: lg-icx

That will cause the "build_openimageio.bash" script used by the CI setup to pull MY branch instead of the master of the official repo.

lgritz · 2022-03-07T19:12:20Z

If you would like to test your osl-side build CI against my in-review OIIO-side changes, you can add these lines to the workflows/ci.yml in the env section of the icx test (and remove it before we're ready for the final merge):

Scratch that. I've merged the changes now, so current OIIO master should give you a clean compile with icx.

… they have version #'s not just 0 or 1, renamed OSL_INTEL_COMPILER to OSL_INTEL_CLASSIC_COMPILER_VERSION renamed OSL_INTEL_LLVM_COMPILER to OSL_INTEL_LLVM_COMPILER_VERSION Removed unnecessary FMT_VERSION and OPENIMAGEIO_CMAKE_FLAGS from the icx-vp2022 worklow, added back USE_SIMD. Fix spelling of losing. Added OSL_OMP_COMPLEX_SIMD_LOOP(...) which is enabled or disabled in platform.h based on which compilers/versions can handle vectorizing complex nested control flow. This makes it cleaner to document which loops need to vectorization conditionally controlled and centralizes where it is managed. Signed-off-by: Alex M. Wells <[email protected]>

Signed-off-by: Alex M. Wells <[email protected]>

lgritz · 2022-03-08T02:20:00Z

I see this is passing all the tests now! Is it ready for review/merge?

AlexMWells · 2022-03-08T17:28:54Z

Yes, its ready for review/merge.

AlexMWells added 3 commits March 4, 2022 17:34

Added icx-vp2022 to CI plan.

e777741

Signed-off-by: Alex M. Wells <[email protected]>

Fix indentation issue in ci.yml

ee4c585

Signed-off-by: Alex M. Wells <[email protected]>

lgritz approved these changes Mar 5, 2022

View reviewed changes

lgritz reviewed Mar 6, 2022

View reviewed changes

AlexMWells added 4 commits March 7, 2022 15:09

Removed extra fp-model=consistent for icx-vp2022 workflow

cf90281

Signed-off-by: Alex M. Wells <[email protected]>

Fix some cut and paste issues replacing vec_width with __OSL_WIDTH

e1a82e1

Signed-off-by: Alex M. Wells <[email protected]>

clang-format wide.h

a95537a

Signed-off-by: Alex M. Wells <[email protected]>

lgritz merged commit 2ab1c5e into AcademySoftwareFoundation:main Mar 8, 2022

	# reciprocal division loosing precision causing slightly different
	# reciprocal division losing precision causing slightly different

		# define OSL_INTEL_LLVM_COMPILER __INTEL_LLVM_COMPILER
		# define OSL_INTEL_CLANG_VERSION (10000__clang_major__ + 100__clang_minor__ + __clang_patchlevel__)

Pr/ci add icx #1473

Pr/ci add icx #1473

Conversation

AlexMWells commented Mar 5, 2022

Description

Tests

Checklist:

lgritz 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

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgritz commented Mar 6, 2022

lgritz commented Mar 6, 2022

lgritz commented Mar 7, 2022

lgritz commented Mar 8, 2022

AlexMWells commented Mar 8, 2022