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 (WW01 2025) #16497

Merged
merged 1,309 commits into from
Jan 2, 2025
Merged

Conversation

iclsrc
Copy link
Contributor

@iclsrc iclsrc commented Dec 31, 2024

ParkHanbum and others added 30 commits December 20, 2024 10:44
…g when length is different (#120461)

insertelt DestVec, (fneg (extractelt SrcVec, Index)), Index
-> shuffle DestVec, (shuffle (fneg SrcVec), poison, SrcMask), Mask

Original combining left the combine between vectors of different lengths as a TODO. this commit do that. (see
#[llvm/llvm-project@baab4aa])
Implementation details:
The UNTIED clause is recognized by setting the flag=0 for the default
case or performing logical OR to flag if other clauses are specified,
and this flag is passed as an argument to the `__kmpc_omp_task_alloc`
runtime call.
…114541)

This patch implements a DAG combine whereby
```
        a: v2i64 = ...
      b: i64 = extract_vector_elt a, Constant:i64<n>
    c: i32 = truncate b
```
Becomes
```
        a: v2i64 = ...
      b: v4i32 = AArch64ISD::NVCAST a
    c: i32 = extract_vector_elt c, Constant:i64<2n>
```

The primary goal of this work is to enable the use of [INS
(element)](https://developer.arm.com/documentation/ddi0602/2024-09/SIMD-FP-Instructions/INS--element---Insert-vector-element-from-another-vector-element-?lang=en)
when moving a truncated i32 element between vectors. This combine
canonicalises the structure of the DAG for all legal instances of the
pattern above (by removing the explicit `trunc` operator in this
specific case), allowing us to take advantage of existing ISEL patterns
for this behavior.
This PR is in reference to porting LLDB on AIX.

Link to discussions on llvm discourse and github:

1. https://discourse.llvm.org/t/port-lldb-to-ibm-aix/80640
2. llvm/llvm-project#101657
The complete changes for porting are present in this draft PR:
llvm/llvm-project#102601

Adding changes for minimal build for lldb binary on AIX. ptrace64 is
needed to debug 64-bit AIX debuggee, and its format is different than
the traditional ptrace on other platforms: [AIX
ptrace](https://www.ibm.com/docs/en/aix/7.3?topic=p-ptrace-ptracex-ptrace64-subroutine)
…0607)

This PR is in reference to porting LLDB on AIX.

Link to discussions on llvm discourse and github:
1. https://discourse.llvm.org/t/port-lldb-to-ibm-aix/80640
2. llvm/llvm-project#101657
The complete changes for porting are present in this draft PR:
llvm/llvm-project#102601

Adding changes for minimal build for lldb binary on AIX: 
The `struct winsize` needed by `lldb/tools/driver/Driver.cpp` is only
recognised in AIX under the AIX specific `_ALL_SOURCE` macro, hence its
enablement is required here.
… (#119694)

This change is related merging some of the APIs in HostInfoLinux into
HostInfoPosix.

Here is the reference PR comment:

llvm/llvm-project#117906 (comment),
llvm/llvm-project#117906 (comment)
…sing (#120279)

When given a DIE for an Objective-C interface (which doesn't have a
`DW_AT_APPLE_objc_complete_type`), the `DWARFASTParserClang` will try to
find the DIE which corresponds to the implementation to complete the
interface DIE. The code is here:

https://github.com/llvm/llvm-project/blob/d2e7ee77d33e8b3be3b1d4e9bc5bc4c60b62b554/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L1718-L1738

However, this was currently not exercised in our test-suite (removing
the code above didn't fail any LLDB test).

This patch adds a test which exercises this codepath (it will fail if we
don't fetch the implementation DIE in the `DWARFASTParserClang`).

Something that's not currently clear to me is why `frame var *f`
succeeds even without the `DW_AT_APPLE_objc_complete_type`
infrastructure. If it's using the ObjC runtime, we should make `expr` do
the same, in which case we can remove this code from
`DWARFASTParserClang`.
…ated mode instead of … (#120424)

…generating empty location

Fix the regression detected by
llvm/llvm-test-suite#188
Since llvm/llvm-test-suite#182 and
llvm/llvm-test-suite#188, these projects can now
be added as external projects within llvm-test-suite.
…r of bools (#118186)

Fixes #116932 

- Remove the quotation marks in the diagnostic message for
err_ext_vector_component_name_illegal
- Pass in the quotation marks directly when reporting an illegal vector
component name inside `CheckExtVectorComponent`
- Add an offset to the `OpLoc` passed into `S.Diag` so the error message
arrow points directly to the offending illegal component rather than to
the '.' at the start of the component identifier.
- Modify the `vector-bool.cpp` element-wise access test case so it
(correctly) now only expects a single set of quotes.
Do not run `cf-to-llvm` as part of `func-to-llvm`. This commit fixes
llvm/llvm-project#70982.

This commit changes the way how `func.func` ops are lowered to LLVM.
Previously, the signature of the entire region (i.e., entry block and
all other blocks in the `func.func` op) was converted as part of the
`func.func` lowering pattern.

Now, only the entry block is converted. The remaining block signatures
are converted together with `cf.br` and `cf.cond_br` as part of
`cf-to-llvm`. All unstructured control flow is not converted as part of
a single pass (`cf-to-llvm`). `func-to-llvm` no longer deals with
unstructured control flow.

Also add more test cases for control flow dialect ops.

Note: This PR is in preparation of #120431, which adds an additional
GPU-specific lowering for `cf.assert`. This was a problem because
`cf.assert` used to be converted as part of `func-to-llvm`.

Note for LLVM integration: If you see failures, add
`-convert-cf-to-llvm` to your pass pipeline.
> See [developer
policy](https://llvm.org/docs/DeveloperPolicy.html#maintainers) for
context on the maintainers terminology.

We currently list @majnemer as the maintainer for InstCombine. While
David does still occasionally contribute in this area, most of the
contributions/reviews come from other people nowadays.

I'd like to propose @dtcxzyw and myself as the new maintainers for this
area. I've also expanded it to include InstSimplify and ValueTracking,
and these tend to all go together.
This commit should have been part of #120580.
`R2` should be always greater than `R1` here because both `R1` and `R2` are not modified inside the loop.
…ng-conversions (#111510) (#118209)

This PR improves the docs for this check to include an example of hidden
narrowing conversions from the integer promotion rules in arithmetic.
…ch + cost-comparison

Helps with debugging to show to that the fold found the match, and shows the old + new costs to indicate whether the fold was/wasn't profitable.
…ssage to help finding vectorcombine stages in the debug log
…ed (#120102)

This is part 1 of caching for smart pointer accessors, building on top
of the CachedConstAccessorsLattice, which caches "normal" accessors.

Smart pointer accessors are a bit different in that they may:
- have aliases to access the same underlying data (but potentially
  returning slightly different types like `&` vs `*`). Within a
  "checked" sequence users may mix uses of the different aliases and the
  check should apply to any of the spellings.
- may have non-const overloads in addition to the const version, where
  the non-const doesn't actually modify the container

Part 2 will follow and add transfer functions utilities. It will also
add a user UncheckedOptionalAccessModel. We'd seen false positives when
nesting StatusOr<optional<T>> and optional<StatusOr<T>>, etc. which this
can help address.
This PR is motivated by a mismatch we discovered between compilation
results with vs. without `-g3`. We noticed this when compiling SPEC2017
testcases. The specific instance we saw is fixed in this PR by modifying
a guard (see below), but it is likely similar instances exist elsewhere
in the codebase.

The specific case fixed in this PR manifests itself in the `SimplifyCFG`
pass doing different things depending on whether DebugInfo is generated
or not. At the end of this comment, there is reduced example code that
shows the behavior in question.

The differing behavior has two root causes:
1. Commit llvm/llvm-project@c07e19b adds loop
metadata including debug locations to loops that otherwise would not
have loop metadata
2. Commit llvm/llvm-project@ac28efa6c100 adds
a guard to a simplification action in `SImplifyCFG` that prevents it
from simplifying away loop metadata

So, the change in 2. does not consider that when compiling with debug
symbols, loops that otherwise would not have metadata that needs
preserving, now have debug locations in their loop metadata. Thus, with
`-g3`, `SimplifyCFG` behaves differently than without it.

The larger issue is that while debug info is not supposed to influence
the final compilation result, commits like 1. blur the line between what
is and is not debug info, and not all optimization passes account for
this.

This PR does not address that and rather just modifies this particular
guard in order to restore equivalent behavior between debug and
non-debug builds in this one instance.

---

Here is a reduced version of a file from `f526.blender_r` that showcases
the behavior in question:
```C
struct LinkNode;
typedef struct LinkNode {
 struct LinkNode *next;
 void *link;
} LinkNode;

void do_projectpaint_thread_ph_v_state() {
  int *ps = do_projectpaint_thread_ph_v_state;
  LinkNode *node;
  while (do_projectpaint_thread_ph_v_state)
    for (node = ps; node; node = node->next)
      ;
}
```
Compiling this with and without DebugInfo, and then disassembling the
results, leads to different outcomes (tested on SystemZ and X86). The
reason for this is that the `SimplifyCFG` pass does different things in
either case.
`llvm::Error` must be consumed, otherwise it will cause trap during destructor
When assumptions are present `Terms.size()` does not actually count the
number of conditions collected from dominating branches; introduce a
separate counter.

Fixes llvm/llvm-project#120237
jsji added 2 commits January 2, 2025 09:32
…21448)

Before 925471e remove_directories
supports path with slash (instead of backslash).
The ILCreateFromPathW in new implementation requires backslash path,
so the call to remove_directories will fail if the path contains slash.

This is to normalize the path to make sure remove_directories still
support path with slash as well.
@jsji jsji force-pushed the llvmspirv_pulldown branch from 146b598 to df0d11a Compare January 2, 2025 17:34
@jsji jsji temporarily deployed to WindowsCILock January 2, 2025 17:34 — with GitHub Actions Inactive
@jsji jsji temporarily deployed to WindowsCILock January 2, 2025 17:35 — with GitHub Actions Inactive
@jsji jsji temporarily deployed to WindowsCILock January 2, 2025 19:22 — with GitHub Actions Inactive
@jsji jsji temporarily deployed to WindowsCILock January 2, 2025 19:54 — with GitHub Actions Inactive
@jsji
Copy link
Contributor

jsji commented Jan 2, 2025

@jsji jsji self-assigned this Jan 2, 2025
@sarnex
Copy link
Contributor

sarnex commented Jan 2, 2025

/merge

@bb-sycl
Copy link
Contributor

bb-sycl commented Jan 2, 2025

Thu 02 Jan 2025 08:47:00 PM UTC --- Merge failed with error: Comment user is not in authorized group or doesn't have merge right for target branch. Please contact with admins of the repo if there is any question.

@bb-sycl
Copy link
Contributor

bb-sycl commented Jan 2, 2025

Thu 02 Jan 2025 08:47:05 PM UTC --- Start to merge the commit into sycl branch. It will take several minutes.

@sarnex
Copy link
Contributor

sarnex commented Jan 2, 2025

lol wat

@sarnex sarnex merged commit e412dd0 into sycl Jan 2, 2025
37 of 39 checks passed
@jsji
Copy link
Contributor

jsji commented Jan 2, 2025

Thu 02 Jan 2025 08:47:00 PM UTC --- Merge failed with error: Comment user is not in authorized group or doesn't have merge right for target branch. Please contact with admins of the repo if there is any question.

@DoyleLi Another failures. but different message now.

@bb-sycl
Copy link
Contributor

bb-sycl commented Jan 2, 2025

Thu 02 Jan 2025 08:52:26 PM UTC --- Merge the branch in this PR to base automatically. Will close the PR later.

@DoyleLi
Copy link
Contributor

DoyleLi commented Jan 3, 2025

Thu 02 Jan 2025 08:47:00 PM UTC --- Merge failed with error: Comment user is not in authorized group or doesn't have merge right for target branch. Please contact with admins of the repo if there is any question.

@DoyleLi Another failures. but different message now.

Hi @jsji there is a debug job which limit to my account yesterday. I disabled it now.

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.