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) #12255

Merged
merged 348 commits into from
Dec 29, 2023
Merged

LLVM and SPIRV-LLVM-Translator pulldown (WW52) #12255

merged 348 commits into from
Dec 29, 2023

Conversation

sys-ce-bb
Copy link
Contributor

MacDue and others added 30 commits December 21, 2023 17:46
…) (#76168)

Also, for consistency make the ZeroOp lowering switch on the ArmSMETileType,
rather than the element bit width.
Follow up to the discussion from #75258, and serves as an alternate
solution for #74670.

Set the location to Unknown for deduplicated / moved / materialized
constants by OperationFolder. This makes sure that the folded constants
don't end up with an arbitrary location of one of the original ops that
became it, and that hoisted ops don't confuse the stepping order.
This reverts commit 199a0f9.
Fixed the left-shift of signed integer which was causing UB.
… (#75827)

Xcode 14.3.1 seems to have dropped these flags so we are creating unit tests to reproduce the issue.
Depositing value into the lowest byte/word is a common code pattern.
This patch improves the code generation for it to avoid redundant AND
and OR operations.
This patch fixes:

  flang/lib/Optimizer/Transforms/StackArrays.cpp:452:7: error:
  ignoring return value of function declared with 'nodiscard'
  attribute [-Werror,-Wunused-result]
This reverts commit 9f0f558.

Fix expensive checks failure by properly marking register def for ADR.
By looking at whether a global is large instead of looking at the code
model.

This also fixes references to large data in the small code model.

We now always fold any 32-bit offset into the addressing mode with the
large code model since it uses 64-bit relocations.
Extends fold-arith-extf-into-vector-contract.mlir by adding a test case
for scalable vectors.
…own large section name (#74381)"

This reverts commit 19fff85.

Now that explicit large globals are handled properly in the small code model.
Renaming a member variable from "Endoding" to "Encoding".

Also replace inlined code for "isNormalized" with a call to the
function, so that if the definition of normalization ever changes, we
only need to change the one place.
… (#75630)

Improve tests for atomic loads and stores, mainly by testing 128-bit atomic load and store instructions both with and w/out natural alignment.
TestGlobalModuleCache.py, a recently added test, tries to update a
source file in the build directory, but it assumes the file is writable.
In our distributed build and test system, this is not always true, so
the test often fails with a write permissions error.

This change fixes that by setting the permissions on the file to be
writable before attempting to write to it.
It makes them easier to read.
…… (#76184)

… scalar result. (#75820)"

This reverts commit 701f647.

The commit breaks some uses of the 'maxloc' intrinsic.

See PR #75820
This diff speeds up CDSplit by not considering any hot-warm splitting
point that could break a fall-through branch from a basic block to its
most likely successor.

Co-authored-by: spupyrev <spupyrev@fb.com>
…d (#76188)

LLVM ObjectFile currently records the start offsets of sections as the
start of the section header, whereas most other tools (WABT, emscripten,
wasm-tools) record it as the start of the section content, after the
header. This affects binutils tools such as objdump and nm, but not
compilation/assembly (since that is driven by symbols and assembler
labels which already have their values inside the section payload rather
in the header. This patch updates LLVM to match the other tools.
This patch fixes the erroneous multiple-target requirement in Fortran
offloading tests. Additionally, it adds two new variables
(test_flags_clang, test_flags_flang) to lit.cfg so that
compiler-specific flags for Clang and Flang can be specified.

This patch re-lands: #74543. The error was caused by having:
```
config.substitutions.append(("%flags", config.test_flags))
config.substitutions.append(("%flags_clang", config.test_flags_clang))
config.substitutions.append(("%flags_flang", config.test_flags_flang))
```
when instead it has to be:
```
config.substitutions.append(("%flags_clang", config.test_flags_clang))
config.substitutions.append(("%flags_flang", config.test_flags_flang))
config.substitutions.append(("%flags", config.test_flags))
```
because LIT replaces with the first longest sub-string match.
This commit implements conditional compilation for ASan helper code.

As convey to me by @EricWF, string benchmarks with UBSan have been
experiencing significant performance hit after the commit with ASan
string annotations. This is likely due to the fact that no-op ASan code
is not optimized out with UBSan. To address this issue, this commit
conditionalizes the inclusion of ASan helper function bodies using
`#ifdef` directives. This approach allows us to selectively include only
the ASan code when it's actually required, thereby enhancing
optimizations and improving performance.

While issue was noticed in string benchmarks, I expect same overhead
(just less noticeable) in other containers, therefore `std::vector` and
`std::deque` have same changes.

To see impact of that change run `string.libcxx.out` with UBSan and
`--benchmark_filter=BM_StringAssign` or
`--benchmark_filter=BM_StringConstruct`.
…rate passthru operand. (#75682)

ISD::VP_MERGE treats the false operand as the source for elements past
VL. The vmerge instruction encodes 3 registers and treats the vd
register as the source for the tail.

This patch adds a new ISD opcode that models the tail source explicitly.
During lowering we copy the false operand to this operand.

I think we can merge RISCVISD::VSELECT_VL with this new opcode by using
an UNDEF passthru, but I'll save that for another patch.
…w's getopt.h (#76137)

We previously were defining _BSD_SOURCE right before including getopt.h.
However, on mingw-w64, getopt.h is also transitively included by
unistd.h, and unistd.h can be transitively included by many headers
(recently, by some libc++ headers).

Therefore, to be safe, we need to define _BSD_SOURCE before including
any header. Thus do this in CMake.

This fixes llvm/llvm-project#76050.
llvm/llvm-project#73077 added -Wswitch-default
diagnostic but it produced false positives in templates. This PR will
address that. llvm/llvm-project#75943
We can't distinguish UAR and UAS, but by definition
UAR is already UAS.
@jsji jsji marked this pull request as ready for review December 28, 2023 21:05
@jsji jsji requested review from a team and bader as code owners December 28, 2023 21:05
@jsji jsji requested a review from againull December 28, 2023 21:05
@jsji jsji self-assigned this Dec 28, 2023
@jsji
Copy link
Contributor

jsji commented Dec 28, 2023

This is ready for review.

@bader
Copy link
Contributor

bader commented Dec 28, 2023

This is ready for review.

@jsji, 5c5358f changes SYCL runtime, so you should tag @intel/llvm-reviewers-runtime team.
BTW, SYCL runtime is not upstreamed, so I'm surprised that this issue is exposed by LLVM pulldown. Do you know what's going on?

@jsji
Copy link
Contributor

jsji commented Dec 28, 2023

This is ready for review.

@jsji, 5c5358f changes SYCL runtime, so you should tag @intel/llvm-reviewers-runtime team. BTW, SYCL runtime is not upstreamed, so I'm surprised that this issue is exposed by LLVM pulldown. Do you know what's going on?

I don't know either. Note that this is NOT exposed in our GitHub Action CI, it was exposed in the RHEL build.
There are only 2 sycl commits, not touching this file, the code itself was there for a long time, but this is a valid warning.
So what I can guess is this might be exposed due to some build env change, eg: we got dispatched to some server that has slightly different build compiler??

@bader
Copy link
Contributor

bader commented Dec 28, 2023

This is ready for review.

@jsji, 5c5358f changes SYCL runtime, so you should tag @intel/llvm-reviewers-runtime team. BTW, SYCL runtime is not upstreamed, so I'm surprised that this issue is exposed by LLVM pulldown. Do you know what's going on?

I don't know either. Note that this is NOT exposed in our GitHub Action CI, it was exposed in the RHEL build. There are only 2 sycl commits, not touching this file, the code itself was there for a long time, but this is a valid warning. So what I can guess is this might be exposed due to some build env change, eg: we got dispatched to some server that has slightly different build compiler??

I see. CI behavior is strange, but I suggest we make 5c5358f change in a separate PR.
The failure is not caused by LLVM pulldown, so we are making unrelated change.

@jsji
Copy link
Contributor

jsji commented Dec 28, 2023

This is ready for review.

@jsji, 5c5358f changes SYCL runtime, so you should tag @intel/llvm-reviewers-runtime team. BTW, SYCL runtime is not upstreamed, so I'm surprised that this issue is exposed by LLVM pulldown. Do you know what's going on?

I don't know either. Note that this is NOT exposed in our GitHub Action CI, it was exposed in the RHEL build. There are only 2 sycl commits, not touching this file, the code itself was there for a long time, but this is a valid warning. So what I can guess is this might be exposed due to some build env change, eg: we got dispatched to some server that has slightly different build compiler??

I see. CI behavior is strange, but I suggest we make 5c5358f change in a separate PR. The failure is not caused by LLVM pulldown, so we are making unrelated change.

Oh, had another look. This should be due to llvm/llvm-project@898320d, so this is exposed by pulldown.

@jsji
Copy link
Contributor

jsji commented Dec 28, 2023

#12260 created for 5c5358f.

We can review follow up there, but I will keep it here for now, or else if we merge this before #12260, our RHEL CI would be broken.

@jsji jsji force-pushed the llvmspirv_pulldown branch from 5c5358f to c99ffd5 Compare December 28, 2023 23:08
@jsji
Copy link
Contributor

jsji commented Dec 28, 2023

@againull approved #12260, so I remove the fix from this pulldown.

@bader @intel/llvm-gatekeepers This does NOT require review anymore, but we should merge this AFTER #12260.

@bader
Copy link
Contributor

bader commented Dec 29, 2023

The build passes even without 5c5358f. 😕
Good to merge anyway.

@bader
Copy link
Contributor

bader commented Dec 29, 2023

/merge

@bb-sycl
Copy link
Contributor

bb-sycl commented Dec 29, 2023

Fri 29 Dec 2023 02:22:46 AM UTC --- Start to merge the commit into sycl branch. It will take several minutes.

@bb-sycl
Copy link
Contributor

bb-sycl commented Dec 29, 2023

Fri 29 Dec 2023 02:27:08 AM UTC --- Merge the branch in this PR to base automatically. Will close the PR later.

@bb-sycl bb-sycl merged commit 9bfc3ae into sycl Dec 29, 2023
35 of 37 checks passed
@jsji
Copy link
Contributor

jsji commented Dec 29, 2023

The build passes even without 5c5358f. 😕 Good to merge anyway.

The build passed because it already picked up the fix in #12260.

19:49:19  [2023-12-29 00:49:19,469] merge INFO: === stage merge start ===
19:49:19  [2023-12-29 00:49:19,469] merge INFO: top_branch_value is 367166cd3a8a315b178a68ba1ac7000de36cde98

$ git show 367166cd3a8a
commit 367166cd3a8a315b178a68ba1ac7000de36cde98
Author: Jinsong Ji [jinsong.ji@intel.com](mailto:jinsong.ji@intel.com)
Date:   Thu Dec 28 16:12:10 2023 -0800

    [SYCL] Fix maybe-uninitialized warning (#12260)

@bader
Copy link
Contributor

bader commented Dec 29, 2023

The build passes even without 5c5358f. 😕 Good to merge anyway.

The build passed because it already picked up the fix in #12260.

19:49:19  [2023-12-29 00:49:19,469] merge INFO: === stage merge start ===
19:49:19  [2023-12-29 00:49:19,469] merge INFO: top_branch_value is 367166cd3a8a315b178a68ba1ac7000de36cde98

$ git show 367166cd3a8a
commit 367166cd3a8a315b178a68ba1ac7000de36cde98
Author: Jinsong Ji [jinsong.ji@intel.com](mailto:jinsong.ji@intel.com)
Date:   Thu Dec 28 16:12:10 2023 -0800

    [SYCL] Fix maybe-uninitialized warning (#12260)

Pre-commit has started before #12260 was merged. It looks like some build job started after #12260 was merged and it pulled most recent commits from the SYCL branch. I expected pre-commit to use the same SYCL branch commit for all build configuration for consistent results. That's why I was confused. I can imagine a situation when Linux and Windows builds will use different commits from the SYCL branch and generate inconsistent pre-commit reports.
To be honest, I'm not sure if this is a bug or a feature. Sounds like using independent commits for each configuration might be beneficial (like in this case).
@intel/dpcpp-devops-reviewers, FYI.

@jsji
Copy link
Contributor

jsji commented Dec 29, 2023

The build passes even without 5c5358f. 😕 Good to merge anyway.

The build passed because it already picked up the fix in #12260.

19:49:19  [2023-12-29 00:49:19,469] merge INFO: === stage merge start ===
19:49:19  [2023-12-29 00:49:19,469] merge INFO: top_branch_value is 367166cd3a8a315b178a68ba1ac7000de36cde98

$ git show 367166cd3a8a
commit 367166cd3a8a315b178a68ba1ac7000de36cde98
Author: Jinsong Ji [[jinsong.ji@intel.com](mailto:jinsong.ji@intel.com)](mailto:[jinsong.ji@intel.com](mailto:jinsong.ji@intel.com))
Date:   Thu Dec 28 16:12:10 2023 -0800

    [SYCL] Fix maybe-uninitialized warning (#12260)

Pre-commit has started before #12260 was merged. It looks like some build job started after #12260 was merged and it pulled most recent commits from the SYCL branch. I expected pre-commit to use the same SYCL branch commit for all build configuration for consistent results. That's why I was confused. I can imagine a situation when Linux and Windows builds will use different commits from the SYCL branch and generate inconsistent pre-commit reports. To be honest, I'm not sure if this is a bug or a feature. Sounds like using independent commits for each configuration might be beneficial (like in this case). @intel/dpcpp-devops-reviewers, FYI.

Yes, it would be great if all the jobs triggered by the same commit/PR use the same top_branch_value.
But since this involve two CI systems, we might need to figure out ways to pass the top_branch_value.
FYI. @DoyleLi

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.