[BACKEND] Update LLVM version to llvm/llvm-project@9ddfe62

[ThomasRaoux]

No description provided.

[gflegar]

@ThomasRaoux we can continue the discussion from #4212 here.

I'm trying to figure out at which point in our internally-known good LLVM commits the problem started happening. So I rebased the llvm-head branch on top of main and re-applied all the updates we've accumulated in the last few weeks one by one, so I get LLVM builds for all of them. I'll create draft PRs once that's done and we can see when it started failing, and merge the updates up to that point.

I can also try to get the latest LLVM version we have internally in the hopes that the issue got fixed in the meantime (will do that once all the builds are done).

In the meantime, I found where OpenXLA (which uses Triton as a dependecy) updated to this exact LLVM version that we know is causing problems in Triton, trying to see if we applied any patches on our side to make it work: openxla/xla@2b7f566
The patches we applied on top of LLVM do not seem relevant to Triton though. 🤷

[ThomasRaoux]

@ThomasRaoux we can continue the discussion from #4212 here.

I'm trying to figure out at which point in our internally-known good LLVM commits the problem started happening. So I rebased the llvm-head branch on top of main and re-applied all the updates we've accumulated in the last few weeks one by one, so I get LLVM builds for all of them. I'll create draft PRs once that's done and we can see when it started failing, and merge the updates up to that point.

I can also try to get the latest LLVM version we have internally in the hopes that the issue got fixed in the meantime (will do that once all the builds are done).

In the meantime, I found where OpenXLA (which uses Triton as a dependecy) updated to this exact LLVM version that we know is causing problems in Triton, trying to see if we applied any patches on our side to make it work: openxla/xla@2b7f566 The patches we applied on top of LLVM do not seem relevant to Triton though. 🤷

Wow this is awesome, thanks a lot! I tried to debug it a bit locally, I realized it happens when I build locally on linux but on my mac when I build locally all the lit tests pass. So it may be some undefined behavior or something that shows up only on some build config :( Hopefully narrowing down the LLVM hash will help figure it out.

[ThomasRaoux]

@gflegar sorry for jumping between different PRs for discussion :) So if I understand correctly this version fails and the version in between wouldn't fail llvm build on ARM?

[gflegar]

@gflegar sorry for jumping between different PRs for discussion :) So if I understand correctly this version fails and the version in between wouldn't fail llvm build on ARM?

No worries, it's a bit complicated with all the different PRs :)

This version passes LLVM builds, but fails on tests. Also, I created #4367, which is currently basically a copy of this PR, but from a separate, locked branch, since the contents of this PR would change any time we try to do something on the llvm-head branch (e.g., once Florian CCed below pushes the new LLVM version to the branch).
The version that fails MacoOS ARM LLVM builds is the one from #4366. I'm running tests on it now (since it does build on non-ARM Ubuntu) to see if we can also reproduce the failing tests there.

Also @reichlfl FYI take a look at discussions here, since you're our next Triton integrator at Google. The TL;DR is that we had CI issues for the last few weeks upstream, and now are trying to catch up, but the latest version of LLVM is causing test failures upstream, but not internally at Google. We're trying to root-cause it. You should still do the integration as normal, and create a PR for the latest LLVM we have internally, so we can check if the issue has maybe already been resolved upstream.

[gflegar]

Alright, it seems that #4366 is even more broken - plenty of tests failing with different errors, one among those is the <<UNKNOWN SSA VALUE>> we've seen in this PR, but that's not the only issue. So maybe the error is coming from somewhere between llvm/llvm-project@4713bd4 and llvm/llvm-project@de88b2c? (It is possible it's a completely unrelated thing, since more stuff is failing on #4366.)

Let's first see if all of this is fixed in the latest version of LLVM we'll roll out this week and save ourselves some time if it is :)

Wow this is awesome, thanks a lot! I tried to debug it a bit locally, I realized it happens when I build locally on linux but on my mac when I build locally all the lit tests pass. So it may be some undefined behavior or something that shows up only on some build config :( Hopefully narrowing down the LLVM hash will help figure it out.

My pleasure! Interesting, maybe it is UB. FWIW we do run Triton's lit tests internally through undefined behavior sanitizer, and that has not detected any issues with this LLVM version (but we do probably run a slightly different build configuration, since we don't use Triton's CMake build system and roll our own Bazel-like thing instead).

[karupayun]

The build starts to fail with "the << UNKNOWN SSA VALUE >> error" after this commit: llvm/llvm-project@ce80c80. It's quite a simple commit, maybe you have a better idea why using a non-deterministic seed could be causing the issue for you.

[ThomasRaoux]

The build starts to fail with "the << UNKNOWN SSA VALUE >> error" after this commit: llvm/llvm-project@ce80c80. It's quite a simple commit, maybe you have a better idea why using a non-deterministic seed could be causing the issue for you.

my first guess is that this header is included in both triton and LLVM but the preprocessor takes a different path on each of those therefore there would be a mismatch in the seeds based on whether the function is built with Triton or with LLVM.
We could verify that by setting LLVM_ENABLE_ABI_BREAKING_CHECKS to 0 for LLVM build?

[gflegar]

That would explain why we're not seeing it internally. We build both LLVM and Triton as one big package, rather than separate binaries, so we wouldn't have this mismatch.

[ThomasRaoux]

BTW, looking at the cmake file of llvm it seems like it will override the pre-processor and the right way to set this cmake option:

set(LLVM_ABI_BREAKING_CHECKS "WITH_ASSERTS" CACHE STRING
  "Enable abi-breaking checks.  Can be WITH_ASSERTS, FORCE_ON or FORCE_OFF.")

here is the cmake logic:

string(TOUPPER "${LLVM_ABI_BREAKING_CHECKS}" uppercase_LLVM_ABI_BREAKING_CHECKS)

if( uppercase_LLVM_ABI_BREAKING_CHECKS STREQUAL "WITH_ASSERTS" )
  if( LLVM_ENABLE_ASSERTIONS )
    set( LLVM_ENABLE_ABI_BREAKING_CHECKS 1 )
  endif()
elseif( uppercase_LLVM_ABI_BREAKING_CHECKS STREQUAL "FORCE_ON" )
  set( LLVM_ENABLE_ABI_BREAKING_CHECKS 1 )
elseif( uppercase_LLVM_ABI_BREAKING_CHECKS STREQUAL "FORCE_OFF" )
  # We don't need to do anything special to turn off ABI breaking checks.
elseif( NOT DEFINED LLVM_ABI_BREAKING_CHECKS )
  # Treat LLVM_ABI_BREAKING_CHECKS like "FORCE_OFF" when it has not been
  # defined.
else()
  message(FATAL_ERROR "Unknown value for LLVM_ABI_BREAKING_CHECKS: \"${LLVM_ABI_BREAKING_CHECKS}\"!")
endif()

[karupayun]

BTW, looking at the cmake file of llvm it seems like it will override the pre-processor and the right way to set this cmake option:

set(LLVM_ABI_BREAKING_CHECKS "WITH_ASSERTS" CACHE STRING
  "Enable abi-breaking checks.  Can be WITH_ASSERTS, FORCE_ON or FORCE_OFF.")

here is the cmake logic:

string(TOUPPER "${LLVM_ABI_BREAKING_CHECKS}" uppercase_LLVM_ABI_BREAKING_CHECKS)

if( uppercase_LLVM_ABI_BREAKING_CHECKS STREQUAL "WITH_ASSERTS" )
  if( LLVM_ENABLE_ASSERTIONS )
    set( LLVM_ENABLE_ABI_BREAKING_CHECKS 1 )
  endif()
elseif( uppercase_LLVM_ABI_BREAKING_CHECKS STREQUAL "FORCE_ON" )
  set( LLVM_ENABLE_ABI_BREAKING_CHECKS 1 )
elseif( uppercase_LLVM_ABI_BREAKING_CHECKS STREQUAL "FORCE_OFF" )
  # We don't need to do anything special to turn off ABI breaking checks.
elseif( NOT DEFINED LLVM_ABI_BREAKING_CHECKS )
  # Treat LLVM_ABI_BREAKING_CHECKS like "FORCE_OFF" when it has not been
  # defined.
else()
  message(FATAL_ERROR "Unknown value for LLVM_ABI_BREAKING_CHECKS: \"${LLVM_ABI_BREAKING_CHECKS}\"!")
endif()

Yes, thanks for pointing it out. Your suggestion works and tests passes when setting the flag. I did not have a lot of time this week but I will be looking next week on how to properly fix it.

[ThomasRaoux]

Yes, thanks for pointing it out. Your suggestion works and tests passes when setting the flag. I did not have a lot of time this week but I will be looking next week on how to properly fix it.

Awesome! Thanks

[MaskRay]

Say my llvm-project build is at /tmp/Rel. /tmp/Rel/include/llvm/Config/abi-breaking.h defines LLVM_ABI_BREAKING_CHECKS.

llvm/include/llvm/ADT/Hashing.h includes abi-breaking.h and gets the LLVM_ABI_BREAKING_CHECKS value from the LLVM build. Whether or not the user build has a NDEBUG mismatch doesn't matter.

If there are Triton failures when the LLVM_ABI_BREAKING_CHECKS==1 code path is used, you might want to check code/tests overly depending on the determinism of llvm/include/llvm/ADT/Hashing.h, which is not guaranteed.

[karupayun]

Created #4512 to set LLVM_ABI_BREAKING_CHECKS=FORCE_OFF as it was suggested here. It's the same we are doing inside Google and I think that being able to update llvm is a priority.

If @ThomasRaoux is interested, in parallel we can replace some DenseMap (and DenseSet) by VectorMap (and VectorSet) to fix the failing unit tests. These structures come with performance overhead, so let me know if you are interested on doing that refactor. Alternative we can investigate what are the issues in the current uses (of DenseMap and DenseSet).

[ThomasRaoux]

If @ThomasRaoux is interested, in parallel we can replace some DenseMap (and DenseSet) by VectorMap (and VectorSet) to fix the failing unit tests. These structures come with performance overhead, so let me know if you are interested on doing that refactor. Alternative we can investigate what are the issues in the current uses (of DenseMap and DenseSet).

As I mentioned on the other PR I don't understand what cases should be changed and why

[ThomasRaoux]

@karupayun which PR do you want to land for the LLVM upgrade?

[ThomasRaoux]

Closing this, we can start a fresh LLVM upgrade now that the problem is fixed.