Skip to content

LLVM and SPIRV-LLVM-Translator pulldown (WW37) #11185

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

Merged
merged 920 commits into from
Sep 15, 2023
Merged

LLVM and SPIRV-LLVM-Translator pulldown (WW37) #11185

merged 920 commits into from
Sep 15, 2023

Conversation

sys-ce-bb
Copy link
Contributor

zeroomega and others added 30 commits September 12, 2023 11:05
This patch adds a missing header to sanitizer_lzw_test to fix a build
breakage after 54c1a9b is landed.
…at token is not an annotation token

In Parser::ParseDirectDeclarator(...) in some cases ill-formed code can cause an
annotation token to end up where it was not expected. The fix is to add a
!Tok.isAnnotation() guard before attempting to access identifier info.

This fixes: llvm/llvm-project#64836

Differential Revision: https://reviews.llvm.org/D158804
- Move the multiprocessing.Pool initializer to a top-level function, it
  was previously causing a pickle failure with my machine's python.
- Change the `env python` to `env python3` for convenience
The builtin_expect(), and C++20's likely, unlikely attributes assign branch_weights to annotated branches.

This patch adds the the ability to query branch !prof metadata and improve static analysis based on that.

Fixes: llvm/llvm-project#64998

Reviewers: tejohnson, efriedma

Differential Revision: https://reviews.llvm.org/D159336
Splits the cleanup block lowered from AsyncToAsyncRuntime.

The incentive of this change is to clarify the CFG branched by
`async.coro.suspend`.

The `async.coro.suspend` op branches into 3 blocks, depending on the
state of the coroutine:
1) suspend
2) resume
3) cleanup

The behavior before this change is that after the coroutine is resumed
and completed, it will jump to a shared cleanup block for destroying the
states of coroutines. The CFG looks like the following,

Entry block
        |                    \
   resume             |
        |                    |
            Cleanup
                   |
                End

This CFG can potentially be problematic, because the `Cleanup` block is
a shared block and it is not dominated by `resume`. For instance, if
some pass wants to add some specific cleanup mechanism to resume, it can
be confused and add them to the shared `Cleanup`, which leads to the
"operand not dominate its use" error because of the existence of the
other "Entry->cleanup" path.

After this change, the CFG will look like the following,

The overall structure of the lowered CFG can be the following,

  Entry (calling async.coro.suspend)
       |                    \
  Resume           Destroy (duplicate of Cleanup)
       |                     |
  Cleanup             |
       |                    /
     End (ends the corontine)

In this case, the Cleanup block tied to the Resume block will be
isolated from the other path and it is strictly dominated by "Resume".
This reverts commit 54c1a9b.
It breaks a few llvm bots.
Also, we removed CMP0076 exception sometime back but did not adjust the
build rules. The adjustment in the build rules is also done in this
patch.
The safe mode is in-between the hardened and the debug modes, extending
the checks contained in the hardened mode with certain checks that are
relatively cheap and prevent common sources of errors but aren't
security-critical. Thus, the safe mode trades off some performance for
a wider set of checks, but unlike the debug mode, it can still be used
in production.

Differential Revision: https://reviews.llvm.org/D158823
This patch fixes:

  llvm/lib/Target/AMDGPU/SIISelLowering.cpp:2493:33: error: unused
  variable 'UserSGPRInfo' [-Werror,-Wunused-variable]
The FileIndex values returned from GetFileInformationByHandle are
considered stable and uniquely identifying a file, as long as the
handle is open. When handles are closed, there are no guarantees
for their stability or uniqueness. On some file systems (such as
NTFS), the indices are documented to be stable even across handles.
But with some file systems, in particular network mounts, file
indices can be reused very soon after handles are closed.

When such file indices are used for LLVM's UniqueID, files are
considered duplicates as soon as the filesystem driver happens to
have used the same file index for the handle used to inspect the
file. This caused widespread, non-obvious (seemingly random)
breakage. This can happen e.g. if running on a directory that is
shared via Remote Desktop or VirtualBox.

To avoid the issue, use a hash of the canonicalized path for the
file as unique identifier, instead of using FileIndex.

This fixes llvm/llvm-project#61401 and
llvm/llvm-project#22079.

Performance wise, this adds (usually) one extra call to
GetFinalPathNameByHandleW for each call to getStatus(). A test
cases such as running clang-scan-deps becomes around 1% slower
by this, which is considered tolerable.

Change the equivalent() function to use getUniqueID instead of
checking individual file_status fields. The
equivalent(Twine,Twine,bool& result) function calls status() on
each path successively, without keeping the file handles open,
which also is prone to such false positives. This also gets rid
of checks of other superfluous fields in the
equivalent(file_status, file_status) function - the unique ID of
a file should be enough (that is what is done for Unix anyway).

This comes with one known caveat: For hardlinks, each name for
the file now gets a different UniqueID, and equivalent() considers
them different. While that's not ideal, occasional false negatives
for equivalent() is usually that fatal (the cases where we strictly
do need to deduplicate files with different path names are quite
rare) compared to the issues caused by false positives for
equivalent() (where we'd deduplicate and omit totally distinct files).

The FileIndex is documented to be stable on NTFS though, so ideally
we could maybe have used it in the majority of cases. That would
require a heuristic for whether we can rely on FileIndex or not.
We considered using the existing function is_local_internal for that;
however that caused an unacceptable performance regression
(clang-scan-deps became 38% slower in one test, even more than that
in another test).

Differential Revision: https://reviews.llvm.org/D155579
It's needed by compiler-rt/test/profile/instrprof-gc-sections.c, which
became active after 2344a72.
…x (#65483)

Fix for an issue where clang was not adding the address space according
to the data layout, instead was using the default which resulted in a
crash at times. The fix includes changes to the cases of
LargeCapMemAlloc and CGroupMemAlloc where we are setting the AddrSpace
according to the DataLayout.
Summary:
We currently call the GPU routine to terminate the current thread in
three separate locations .This should be wrapped into a helper function
to simplify the implementation.
This cleans up a unnecessary code that changes zero size allocation to
avoid the following error message

'cuMemAlloc(&ptr, sizeBytes)' failed with 'CUDA_ERROR_INVALID_VALUE'
Previously, annotations were only emitted for function definitions. With
this change annotations are also emitted for declarations. Also, emitting
function annotations is now deferred until the end so that the most
up to date declaration is used which will have any inherited annotations.

Differential Revision: https://reviews.llvm.org/D156172/new/
This loop is wrong, most of targets are not defined yet.
Also if we build with LLVM_ENABLE_RUNTIMES, these deps
are irrelevant.
…uests (#66037)

This will reduce the number of notifications created when a pull request
label is added. Each team will only get a notification when their team's
label is added and not when other teams' labels are added.
Make codegen emit correctly rounded sqrt by default.

Emit the fast but only kind of fast expansion in AMDGPUCodeGenPrepare
based on !fpmath, like the fdiv case. Hack around visitation ordering
problems from AMDGPUCodeGenPrepare using forward iteration instead of
a well behaved combiner.

https://reviews.llvm.org/D158129
We want the !fpmath metadata to be attached to the sqrt intrinsic to
make it to the backend lowering. Emit an available_externally
definition which uses the builtin, which emits the !fpmath.

Fixes #64264

https://reviews.llvm.org/D156743
These tests are still relatively flaky on certain buildbots and on some
developer machines. This patch hopefully reduces the noise produced by
these tests in those environments by setting the retry count to 2 so
that they will hopefully flaky pass rather than just fail while I
perform more investigation.
It's weird that we're specifying this in both the dxil and the LLVM IR
way, but if we are we should at least be consistent about it.
This patch ports the AMDGPURewriteUndefForPHI pass to the new pass
manager. With this, the pass is supported under both the legacy and the
new pass managers.

---------

Co-authored-by: Jun Wang <[email protected]>
…(#66116)

Propagate "branch_weights" metadata whe turning a select into a
conditional branch in tryToUnfoldSelectInCurrBB
@sys-ce-bb sys-ce-bb added the disable-lint Skip linter check step and proceed with build jobs label Sep 14, 2023
@jsji jsji closed this Sep 14, 2023
@jsji jsji reopened this Sep 14, 2023
@jsji jsji marked this pull request as ready for review September 14, 2023 20:17
@jsji jsji requested review from a team and bader as code owners September 14, 2023 20:17
@jsji
Copy link
Contributor

jsji commented Sep 14, 2023

This is ready for review, 3 fixes:

@jsji jsji self-assigned this Sep 14, 2023
@jsji
Copy link
Contributor

jsji commented Sep 14, 2023

@aelovikov-intel Looks like there are some workflow files coming from community, eg: Check Python Formatting / python_formatting (pull_request) , PR Receive Label / pr-subscriber (pull_request) Do we want to remove those?

@aelovikov-intel
Copy link
Contributor

@aelovikov-intel Looks like there are some workflow files coming from community, eg: Check Python Formatting / python_formatting (pull_request) , PR Receive Label / pr-subscriber (pull_request) Do we want to remove those?

It seems the upstream is still actively tweaking their CI, I'd rather let it settle first.

@jsji
Copy link
Contributor

jsji commented Sep 14, 2023

It seems the upstream is still actively tweaking their CI, I'd rather let it settle first.

OK. Thanks.

@bader
Copy link
Contributor

bader commented Sep 14, 2023

I quickly skimmed through these patches:

Tagging @npmiller as this is NVPTX specific patch. Should this fix be done in upstream? It doesn't seem to be specific to SYCL, so I hope we don't have NVPTX customizations in the sycl branch. AFAIK, Codeplay team makes NVPTX back-end fixes in llorg directly.

This also doesn't seem like a change directly related to SYCL, so we should expect it to be done in upstream too. Am I right?
@andykaylor, is this related to #8134?

This looks reasonable.

@jsji
Copy link
Contributor

jsji commented Sep 14, 2023

Tagging @npmiller as this is NVPTX specific patch. Should this fix be done in upstream? It doesn't seem to be specific to SYCL, so I hope we don't have NVPTX customizations in the sycl branch. AFAIK, Codeplay team makes NVPTX back-end fixes in llorg directly.

Right, this has been done in upstream too, in 86735a4. I fixed it before we merged upstream fix. We merged 86735a4 later too, no conflict, exact same change, so no review needed anymore. Thanks.

This also doesn't seem like a change directly related to SYCL, so we should expect it to be done in upstream too. Am I right? @andykaylor, is this related to #8134?

This is related to #8280. The feature mentioned it is for SYCL. @zahiraam Please review. Thanks.

Copy link
Contributor

@zahiraam zahiraam left a comment

Choose a reason for hiding this comment

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

Changes made in CGBuiltin.cpp for the exp2 builtin look good to me.

@jsji
Copy link
Contributor

jsji commented Sep 15, 2023

@bader @intel/llvm-gatekeepers Ready to merge?

@bader
Copy link
Contributor

bader commented Sep 15, 2023

/merge

@bb-sycl
Copy link
Contributor

bb-sycl commented Sep 15, 2023

Fri 15 Sep 2023 08:04:04 PM UTC --- Start to merge the commit into sycl branch. It will take several minutes.

@bb-sycl
Copy link
Contributor

bb-sycl commented Sep 15, 2023

Fri 15 Sep 2023 08:08:06 PM UTC --- Merge the branch in this PR to base automatically. Will close the PR later.

@bb-sycl bb-sycl merged commit 0989c8a into sycl Sep 15, 2023
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.