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 (WW04) #12495

Merged
merged 858 commits into from
Jan 30, 2024
Merged

LLVM and SPIRV-LLVM-Translator pulldown (WW04) #12495

merged 858 commits into from
Jan 30, 2024

Conversation

sys-ce-bb
Copy link
Contributor

mstorsjo and others added 30 commits January 23, 2024 13:39
In acd8791, a call to FlushFileBuffers
was added to work around a rare kernel bug. In
3b9b4d2, the scope of that workaround
was limited, for performance reasons, as the flushes are quite
expensive.

On VirtualBox shared folders, closing a memory mapping that has been
written to, also needs to be explicitly flushed, if renaming the output
file before it is closed. Contrary to the kernel bug, this always
happens on such mounts. In these cases, the output ends up as a file of
the right size, but the contents are all zeros.

The sequence to trigger the issue on the VirtualBox Shared Folders is
this, summarized:

    file = CreateFile()
    mapping = CreateFileMapping(file)
    mem = MapViewOfFile()
    CloseHandle(mapping)
    write(mem)
    UnmapViewOfFile(mem)
    SetFileInformationByHandle(file, FileRenameInfo)
    CloseHandle(file)

With this sequence, the output file always ends up with all zeros. See
mstorsjo/llvm-mingw#393 for a full
reproduction example.

To avoid this issue, call FlushFileBuffers() when the file may reside on
a VitualBox shared folder. As the flushes are expensive, only do them
when the output isn't on a local file system.

The issue with VirtualBox shared folders could also be fixed by calling
FlushViewOfFile before UnmapViewOfFile, and doing that could be slightly
less expensive than FlushFileBuffers.

Empirically, the difference between the two is very small though, and as
it's not easy to verify whether switching FlushFileBuffers to
FlushViewOfFile helps with the rare kernel bug, keep using
FlushFileBuffers for both cases, for code simplicity.

This fixes downstream bug
mstorsjo/llvm-mingw#393.
These stem from 4821c90.

When cross compiling, CMAKE_SYSTEM_PROCESSOR is empty, if the
target processor hasn't been set when setting up the cross
compilation. Ideally, when setting up cross compilation with
CMake, the user is supposed to set CMAKE_SYSTEM_PROCESSOR, but
so far, compilation has worked just fine even without it.

Quote the string where CMAKE_SYSTEM_PROCESSOR is expanded, to
avoid argument errors when it expands to an empty string.
The constructor args are passed by reference since d6790a0

Fixes MSVC static analysis warning
…g (#78980)

Following on from the previous patch 6aeb7a7,
this patch adds the necessary code to process the DPV equivalents of
llvm.dbg.assign intrinsics. Most of the content of this patch is simply
duplicating existing functionality, using generic code for simple
functions and PointerUnions where storage is required. The most complex
changes are in the places that iterate over instructions, as iterating
over DPValues between instructions is different to iterating over
instructions that may or may not be debug intrinsics; this is most
complex in `AssignmentTrackingLowering::process`, where I've added some
comments to explain the state of the program at each key point depending
on whether we are operating on intrinsics or DPValues.
This is /almost/ NFC - the only annoyance is that for some reason we were using "<C1,C2,..>" for ConstantVector types unlike all other cases - these now use the same "[C1,C2,..]" format as the other constant printers.
3ab8d2a relanded in 3112578, so reland this too.

Might want to set this to True (and add a few source files to
builtins) at some point, but for now heal the bots.
…#79128)

- reland #79113
- Fix aarch64 RISC-V build
These were missed and hopefully avoids assertions when
dc3faf0 is recommitted.
…ang ToT (#77853)

There were quite a few compilation warnings when building openmp on Windows with
the latest Visual Studios 2022 version 17.8.4. Some other warnings were visible
with the latest Clang at tip. This commit fixes all of them.
Replace cp with a cat. This allows to create a writable file when the
original one is read-only.
These are largely copy-pasted from the corresponding function
descriptions. Added \see cross-references. Also changed <c> tags to \c.
So we have all the complex casts together.
Mistake with #78628 that got caught after being merged
…78972)

Make Candidate's front() and back() functions return references to
MachineInstr and introduce begin() and end() returning iterators, the
same way it is usually done in other container-like classes.

This makes possible to iterate over the instructions contained in
Candidate the same way one can iterate over MachineBasicBlock (note that
begin() and end() return bundled iterators, just like MachineBasicBlock
does, but no instr_begin() and instr_end() are defined yet).
…ding lowering (#78982)

This patch adds support for DPVAssigns across all of
AssignmentTrackingAnalysis except for AssignmentTrackingLowering, which
is implemented in a separate patch. This patch includes handling
DPValues in MemLocFragFill, the removal of redundant DPValues as part of
AssignmentTrackingAnalysis (which is different to the version in
`BasicBlockUtils.cpp`), and preventing the DPVAssigns from being
directly emitted in SelectionDAG (just as we don't emit llvm.dbg.assigns
directly, but receive a set of locations from
AssignmentTrackingAnalysis' output).
…(#78995)

LLVM will shortly be able to represent variable locations without
encoding information into intrinsics -- they'll be stored as DPValue
objects instead. We'll still need to be able to llvm-reduce these
variable location assignments just like we can with intrinsics today,
thus, here's an llvm-reduce pass that enumerates and reduces the DPValue
objects.

The test for this is paradoxically written with dbg.value intrinsics:
this is because we're changing all the core parts of LLVM to support
this first, with the textual IR format coming last. Until that arrives,
testing the llvm-reduce'ing of DPValues needs the added test using
intrinsics. We should be able to drop the variable assignment using
%alsoloaded using this method. As with the other llvm-reduce tests, I've
got one set of check lines for making the reduction happen as desired,
and the other set to check the final output.
This introduces the Armv9.5-A architecture version to the Arm backend,
following on from the existing implementation for AArch64 targets.

Mode details about the Armv9.5-A architecture version can be found at:
* https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/arm-a-profile-architecture-developments-2023
* https://developer.arm.com/documentation/ddi0602/2023-09/
This fixes the build since 8f90e69
which made libcxxabi use llvm's libunwind by default.

Fixes #78487
As AMDGPU backend has moved to cov5 as default, mlir should also switch
to it.
…228)

The RemoveDIs project is aiming to eliminate debug intrinsics like
dbg.value and dbg.declare from LLVM, and replace them with DPValue objects
attached to instructions. ISel is one of the "terminals" where that
information needs to be converted into MIR format: this patch implements
support for that in GlobalISel. We aim for the output of LLVM to be
identical with/without RemoveDIs debug-info.

This patch should be NFC, as we're handling the same data about variables
stored in a different format -- it now appears in a DPValue object rather
than as an intrinsic. To that end, I've refactored the handling of
dbg.values into a dedicated function, and call it whenever a dbg.value or a
DPValue is encountered. dbg.declare is handled in a similar way.

Testing: adding the --try-experimental-debuginfo-iterators switch to llc
causes it to try and convert to the "new" debug-info format if it's built
in (LLVM_EXPERIMENTAL_DEBUGINFO_ITERATORS=On), and it'll be covered by our
buildbot. One test has a few extra wildcard-regexes added: this is because
there's some extra data printed about attached debug-info, which is safe to
ignore.
…test (#79150)

llvm/llvm-project#78285 added a test which calls
`__llvm_orderfile_dump()`, a functionality that is not supported on many
platforms. This PR removes the call to `__llvm_orderfile_dump()` to
avoid it failing on unsupported platforms, and turn on the test for
Windows, so we test the rest of the API that are supported.
@jsji jsji requested review from a team and bader as code owners January 29, 2024 17:06
@jsji jsji requested a review from ldrumm January 29, 2024 17:06
@jsji
Copy link
Contributor

jsji commented Jan 29, 2024

@jsji
Copy link
Contributor

jsji commented Jan 29, 2024

Generate Doxygen documentation / build (pull_request)

This is CI failures common to other PR, irrelevant .

@aelovikov-intel
Copy link
Contributor

cuda e2e failure looks real.

@jsji
Copy link
Contributor

jsji commented Jan 29, 2024

cuda e2e failure looks real.

it is the same as AMD HIP failure, I will update the XFAIL then.

@jsji jsji force-pushed the llvmspirv_pulldown branch from d4ce4f3 to e2a8b3e Compare January 29, 2024 18:54
@jsji jsji temporarily deployed to WindowsCILock January 29, 2024 19:36 — with GitHub Actions Inactive
@jsji jsji temporarily deployed to WindowsCILock January 29, 2024 20:15 — with GitHub Actions Inactive
@jsji
Copy link
Contributor

jsji commented Jan 30, 2024

@bader @intel/llvm-gatekeepers Can we get this merged? Thanks.

@bader
Copy link
Contributor

bader commented Jan 30, 2024

@jsji, I'd like code owners to review the changes made on top of community commits.

@jsji
Copy link
Contributor

jsji commented Jan 30, 2024

@jsji, I'd like code owners to review the changes made on top of community commits.

@intel/dpcpp-cfe-reviewers @sommerlukas @YuriPlyakhin Would you please add comment explicitly in this PR so that this can be merged. Thanks.

Copy link
Contributor

@sommerlukas sommerlukas left a comment

Choose a reason for hiding this comment

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

Reviewed commit e2a8b3e.

We're already investigating the issue, but makes sense to XFAIL the test while we investigate.

@jsji
Copy link
Contributor

jsji commented Jan 30, 2024

Reviewed commit e2a8b3e.

We're already investigating the issue, but makes sense to XFAIL the test while we investigate.

Thanks @sommerlukas

Copy link
Contributor

@smanna12 smanna12 left a comment

Choose a reason for hiding this comment

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

CFE changes look good to me.

@YuriPlyakhin
Copy link
Contributor

Reviewed ece5ab1

OK to XFAIL the test, while root cause is not clear.

@jsji
Copy link
Contributor

jsji commented Jan 30, 2024

@bader @intel/llvm-gatekeepers Can we get this merged? Thanks.

@bader
Copy link
Contributor

bader commented Jan 30, 2024

/merge

@bb-sycl
Copy link
Contributor

bb-sycl commented Jan 30, 2024

Tue 30 Jan 2024 04:22:33 PM UTC --- Start to merge the commit into sycl branch. It will take several minutes.

@bb-sycl
Copy link
Contributor

bb-sycl commented Jan 30, 2024

Tue 30 Jan 2024 04:28:11 PM UTC --- Merge the branch in this PR to base automatically. Will close the PR later.

@bb-sycl bb-sycl merged commit 9741bdc into sycl Jan 30, 2024
17 of 18 checks passed
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.