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

LLVM and SPIRV-LLVM-Translator pulldown (WW52 2024) #16484

Merged
merged 1,462 commits into from
Dec 30, 2024
Merged

LLVM and SPIRV-LLVM-Translator pulldown (WW52 2024) #16484

merged 1,462 commits into from
Dec 30, 2024

Conversation

iclsrc
Copy link
Contributor

@iclsrc iclsrc commented Dec 27, 2024

frobtech and others added 30 commits December 9, 2024 11:18
For whatever reason, each ctype test contains its own copy of
some identical helper source code.  These local helpers were
defined with external linkage for no apparent reason.  This leads
to multiple definition errors when linking these tests together.

This change moves each file's local helper code into an anonymous
namespace so it has internal linkage.  It's notable that the libc
test code does not follow the most common norm of gtest-style
code where all the `TEST(...)` cases themselves are defined
inside an anonymous namespace (along with whatever other local
helpers they use); whether libc's tests should follow that usual
convention can be addressed holistically in future discussion.

The replacement of numerous cut&paste'd copies of identical
helper code with sharing the source code in some usual fashion is
also left for later cleanup.

This change only makes the test code not straightforwardly have
multiple definition errors that prevent linking a test executable
at all.
This was added in #117573 but the options were not being rendered
correctly due to the missing newline after `::`.
…#119252)

Reverts llvm/llvm-project#112277

This broke something on Fuchsia's Mac builders, 
so there's still something in the CMake that needs to be updated before
we reland.

Failed build: 

https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-mac-xarm64/b8729005878443108801/overview
… (#117195)" (#119247)

The previous patch
https://github.com/llvm/llvm-project/pull/116860/files#diff-e7e06355c973f68f900d2a34a4103dbfa022589c55c59d02870da9365acf7b98L651

 seems to mistakenly overwrites true16 test lines.

i.e.
```
v_fmaak_f16 v5.l, v1.l, v2.l, 0xfe0b
```
to
```
v_fmaak_f16 v5, v1, v2, 0xfe0b
```

Planned to revert patch
llvm/llvm-project#117195
llvm/llvm-project#116860
and redo these two.

This is the revert of the patch 117195. The revert of 116860 will be in
a seperate patch
Approximates the shadow propagation via OR'ing.

Updates the neon_vmul.ll test introduced in
llvm/llvm-project#117935
…… (#119253)

The previous patch
https://github.com/llvm/llvm-project/pull/116860/files#diff-e7e06355c973f68f900d2a34a4103dbfa022589c55c59d02870da9365acf7b98L651

seems to mistakenly overwrites true16 test lines.

i.e.

v_fmaak_f16 v5.l, v1.l, v2.l, 0xfe0b
to

v_fmaak_f16 v5, v1, v2, 0xfe0b
Planned to revert patch
llvm/llvm-project#117195
llvm/llvm-project#116860
and redo these two.

This is the revert of the patch 116860.
…9202)

This PR improves general validity of emitted code between passes due to
generation of `TargetOpcode::PHI` instead of `SPIRV::OpPhi` after
Instruction Selection, fixing generation of OpTypePointer instructions
and using of proper virtual register classes.

Using `TargetOpcode::PHI` instead of `SPIRV::OpPhi` after Instruction
Selection has a benefit to support existing optimization passes
immediately, as an alternative path to disable those passes that use
`MI.isPHI()`. This PR makes it possible thus to revert
llvm/llvm-project#116060 actions and get back to
use the `MachineSink` pass.

This PR is a solution of the problem discussed in details in
llvm/llvm-project#110507. It accepts an advice
from code reviewers of the PR #110507 to postpone generation of OpPhi
rather than to patch CodeGen. This solution allows to unblock
improvements wrt. expensive checks and makes it unrelated to the general
points of the discussion about OpPhi vs. G_PHI/PHI.

This PR contains numerous small patches of emitted code validity that
allows to substantially pass rate with expensive checks. Namely, the
test suite with expensive checks set ON now has only 12 fails out of 569
total test cases.

FYI @bogner
FWICT, these were the newly added headers for c11.
The functions are not relevant for most sanitizers and only required for
MSan to see which regions have been written to. This eliminates a link
dependency for all other sanitizers and fixes #59007: while `-lresolv`
had been added for the static runtime in 6dce56b, it wasn't added
to the shared runtimes.

Instead of just moving the interceptors, we adapt them to MSan
conventions:
* We don't skip intercepting when `msan_init_is_running` is true, but
directly call ENSURE_MSAN_INITED() like most other interceptors. It
seems unlikely that these functions are called during initialization.
* We don't unpoison `errno`, because none of the functions is specified
to use it.
We do not have CI coverage for Windows/MacOS and we regularly run into
problem where changes break post-commit fullbuild which is not tested in
pre-commit builds. This PR utilizes the github action to address such
issues.
…nc and gpu.func (#119034)

Use `pm.nest` to schedule the pass on nested `func.func` and `gpu.func`
in the `gpu.module`.

AbstractResult pass is not meant to run on the whole gpu.module at once.
Reverts revert #118517 after (hopefully) fixing builders
(llvm/llvm-zorg#328,
llvm/llvm-zorg#327)

This reverts commit 61bf308.
…13474)

Update VOP3dot instructions with true16 and fake16 formats.

This patch includes instructions:
v_dot2_f16_f16
v_dot2_bf16_bf16
…9263)

This is a NFC change

The previous patch llvm/llvm-project#116860 has
an issue and is reverted in
llvm/llvm-project#119253.

Redo the patch here
Lowering `math.powf` to `llvm.intr.powf` will result in `pow(x, 0) =
1`, even for `x=0`. When using the Math dialect expansion patterns,
`pow(0, 0)` will result in `-nan`, however, This change adds two
additional instructions to the lowering to ensure the `pow(x, 0)` case
lowers to to `1` regardless of the value of `x`.

Resolves llvm/llvm-project#118945.
Move code to prepare the VPlan for the epilogue vector loop to a helper
to reduce size and complexity of processLoop.
Looks like we were missing docs for:
- float.h
- wchar.h
- wctype.h

Which AFAICT were added in ISO C99.
Once again, this is a clause on a combined construct that does almost
exactly what the loop/compute construct version does, only with some sl
ightly different evaluation rules/sema rules as it doesn't have to
consider the parent, just the 'combined' construct.  The two sets of
rules for reduction on loop and compute are fine together, so this
ensures they are all enforced for this too.

The 'gangs' 'num_gangs' 'reduction' diagnostic (Dim>1) had to be applied
to num_gangs as well, as it previously wasn't permissible to get in this
situation, but we now can.
Directly check VectorizingEpilogue which directly indicates that the
epilogue is vectorized.
Our byval call lowering isn't copying the argument. Looks like our
SelectionDAG code for byval is different than AArch64 so this may be
non-trivial to fix. Reject for now.
This is a NFC change

The previous patch llvm/llvm-project#117195 has
an issue and is reverted in
llvm/llvm-project#119247

Redo the patch here
This is a NFC change. Update mc test for v_add/sub_f16 in true16 format.

MC source change was done by previous patch and automatically enabled by
t16 pesudo
Update the code to create induction resume PHIs to also create a resume
phi for the canonical induction during epilogue vectorization. This
unifies the code for handling induction resume values and removes the
need to explicitly create manually resume PHI and return it during
epilogue creation.

Overall it helps to move the code for updating the canonical induction
resume value to the place where all other header phi resume values are
updated.

This is NFC, modulo order of the created phis.
The acc data clause operations hold an operand named `varPtr`. This was
intended to hold a pointer to a variable - where the element type of
that pointer specifies the type of the variable. However, for both
memref and llvm dialects, this assumption is not true. This is because
memref element type for cases like memref<10xf32> is simply f32 and for
LLVM, after opaque pointers, the variable type is no longer recoverable.

Thus, introduce varType to ensure that appropriate semantics are kept.

Both the parser and printer for this new type attribute allow it to not
be specified in cases where a dialect's getElementType() applied to
`varPtr`'s type has a recoverable type. And more specifically, for FIR,
no changes are needed in the MLIR unit tests.
…tructuredBuffer` (#118536)

The methods are using existing clang builtins
`__builtin_hlsl_buffer_update_counter` and
`__builtin_hlsl_resource_getpointer` to update the buffer counter and
then load or store the value.

Fixes #112968
We have users that target baremetal aarch64.
@jsji
Copy link
Contributor

jsji commented Dec 27, 2024

@@ -14,7 +14,7 @@
// RUN: %if linux %{ llvm-link -o=%t_app.bc %t.bc %t_compiler_wrappers.bc %t_asan.bc %} %else %{ llvm-link -o=%t_app.bc %t.bc %t_compiler_wrappers.bc %}
// >> ---- translate to SPIR-V
// RUN: llvm-spirv -o %t.spv %t_app.bc
// RUN: %clangxx -Wno-error=ignored-attributes %sycl_include -DSYCL_DISABLE_FALLBACK_ASSERT %cxx_std_optionc++17 %include_option %t.h %s -o %t.out %sycl_options -fno-sycl-dead-args-optimization -Xclang -verify-ignore-unexpected=note,warning
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean "unused"? According to the comment at line 6, it's required for this test.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it's required for the device compilation because host part is compiled in non-sycl mode. -fno-sycl-dead-args-optimization is ignored in non-sycl mode.
Did I get it right? If so, please, update the test comment and commit message.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated commit message with test failures. The option is still used in line 9 after the comments, so I don't think we need to update the comments.

@bader
Copy link
Contributor

bader commented Dec 27, 2024

LLVM: llvm/llvm-project@2fe30bc

@jsji, this patch was committed 17 days ago. Do you know the reason for the delay with pulling llvm-project commits?

Fix failures in https://github.com/intel/llvm/actions/runs/12517379530/job/34918397208

```
/__w/llvm/llvm/toolchain/bin//clang++  -Werror  -Wno-error=ignored-attributes -isystem /__w/llvm/llvm/toolchain/include -DSYCL_DISABLE_FALLBACK_ASSERT -std=c++17 -include /__w/llvm/llvm/build-e2e/Config/Output/kernel_from_file.cpp.tmp.h /__w/llvm/llvm/llvm/sycl/test-e2e/Config/kernel_from_file.cpp -o /__w/llvm/llvm/build-e2e/Config/Output/kernel_from_file.cpp.tmp.out  -lsycl -I/__w/llvm/llvm/toolchain/include -I/__w/llvm/llvm/toolchain/include/sycl -L/__w/llvm/llvm/toolchain/lib -fno-sycl-dead-args-optimization -Xclang -verify-ignore-unexpected=note,warning

clang++: error: argument unused during compilation: '-fno-sycl-dead-args-optimization' [-Werror,-Wunused-command-line-argument]
```
@jsji
Copy link
Contributor

jsji commented Dec 30, 2024

@intel/llvm-gatekeepers This is ready for merge, can someone help issue a '/merge'. Thanks!

@dm-vodopyanov
Copy link
Contributor

dm-vodopyanov commented Dec 30, 2024

I can type '/merge' but not all checks were completed successfully.
@jsji, @intel/dpcpp-devops-reviewers,
IGC DEV CI Containers / Build and Push IGC Dev Docker Images (Intel Drivers Ubuntu 24.04 Docker image with dev IGC, ubunt... (pull_request) failed due to lack of /usr/local/lib/libopencl-clang.so.14*, is it safe to merge, or issue should be addressed before merge?

@sarnex
Copy link
Contributor

sarnex commented Dec 30, 2024

We can ignore the dev igc issue, the current upstream IGC repo has been in a bad state for a long time.

@sarnex
Copy link
Contributor

sarnex commented Dec 30, 2024

/merge

@bb-sycl
Copy link
Contributor

bb-sycl commented Dec 30, 2024

Mon 30 Dec 2024 03:12:27 PM UTC --- Start to merge the commit into sycl branch. It will take several minutes.

@bb-sycl
Copy link
Contributor

bb-sycl commented Dec 30, 2024

Mon 30 Dec 2024 03:12:28 PM UTC --- Merge failed with error: Please check whether the PR is mergeable

@sarnex sarnex merged commit 31a339e into sycl Dec 30, 2024
37 of 38 checks passed
@jsji
Copy link
Contributor

jsji commented Dec 30, 2024

Mon 30 Dec 2024 03:12:28 PM UTC --- Merge failed with error: Please check whether the PR is mergeable

@DoyleLi The bot is failing AGAIN... Can you check what went wrong this time. Thanks.

@jsji jsji deleted the llvmspirv_pulldown branch December 30, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disable-lint Skip linter check step and proceed with build jobs
Projects
None yet
Development

Successfully merging this pull request may close these issues.