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

[vectorization] support forked pointer from loop body with control flow #64888

Closed
vfdff opened this issue Aug 22, 2023 · 2 comments · Fixed by #65834
Closed

[vectorization] support forked pointer from loop body with control flow #64888

vfdff opened this issue Aug 22, 2023 · 2 comments · Fixed by #65834
Assignees

Comments

@vfdff
Copy link
Contributor

vfdff commented Aug 22, 2023

test: https://godbolt.org/z/bcYTW7KoT

void s1161_noReadWrite(int *Preds) {
  for (int i = 0; i < LEN_1D-1; ++i) {
    if (Preds[i] != 0)
      b[i] = c[i] + 1;
    else
      a[i] = i * i;
  }
}

This case is simplified from #64292,
but there is no pointer exists for both read and write, which will skip to check areDepsSafe.
I think this should be first supported.

@vfdff vfdff changed the title [vectorization] support forked phi pointer from loop body with control flow [vectorization] support forked pointer from loop body with control flow Aug 22, 2023
@vfdff vfdff self-assigned this Aug 22, 2023
@vfdff
Copy link
Contributor Author

vfdff commented Aug 23, 2023

try to fix with https://reviews.llvm.org/D158493

@vfdff
Copy link
Contributor Author

vfdff commented Sep 1, 2023

a new implementation solution on D158965

vfdff added a commit to vfdff/llvm-project that referenced this issue Sep 9, 2023
Given a function like the following: https://godbolt.org/z/T9c99fr88
    ```
    1161_noReadWrite(int *Preds) {
      for (int i = 0; i < LEN_1D-1; ++i) {
        if (Preds[i] != 0)
          b[i] = c[i] + 1;
        else
          a[i] = i * i;
      }
    }
    ```

LLVM will optimize the IR to a single store by a phi instruction:

   ```
      %1 = load ptr, ptr @A, align 64
      %2 = load ptr, ptr @b, align 64
      ...
    for.inc:
      %.sink = phi ptr [ %1, %if.then ], [ %2, %if.else ]
      %add.sink = phi double [ %add, %if.then ], [ %conv8, %if.else ]
      %arrayidx7 = getelementptr inbounds double, ptr %.sink, i64 %indvars.iv
      store double %add.sink, ptr %arrayidx7, align 8
   ```

LAA is currently unable to analyze such IR, since ScalarEvolution
will return a SCEVUnknown for the forked pointer operand of the store.

This patch adds initial optional support for analyzing both possibilities for the pointer
and allowing LAA to generate runtime checks for the bounds if required, refers to D108699, but here address the phi node.

Fixes llvm#64888
vfdff added a commit that referenced this issue Sep 19, 2023
Given a function like the following: https://godbolt.org/z/T9c99fr88
```c
1161_noReadWrite(int *Preds) {
  for (int i = 0; i < LEN_1D-1; ++i) {
    if (Preds[i] != 0)
      b[i] = c[i] + 1;
    else
      a[i] = i * i;
  }
}
```

LLVM will optimize the IR to a single store by a phi instruction:

   ```llvm
      %1 = load ptr, ptr @A, align 64
      %2 = load ptr, ptr @b, align 64
      ...
    for.inc:
      %.sink = phi ptr [ %1, %if.then ], [ %2, %if.else ]
      %add.sink = phi double [ %add, %if.then ], [ %conv8, %if.else ]
%arrayidx7 = getelementptr inbounds double, ptr %.sink, i64 %indvars.iv
      store double %add.sink, ptr %arrayidx7, align 8
   ```

LAA is currently unable to analyze such IR, since ScalarEvolution will
return a SCEVUnknown for the forked pointer operand of the store.

This patch adds initial optional support for analyzing both
possibilities for the pointer and allowing LAA to generate runtime
checks for the bounds if required, refers to D108699, but here address
the phi node.

Fixes #64888

Reviewed By: huntergr-arm, fhahn
Differential Revision: https://reviews.llvm.org/D158965
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this issue Sep 19, 2023
Given a function like the following: https://godbolt.org/z/T9c99fr88
```c
1161_noReadWrite(int *Preds) {
  for (int i = 0; i < LEN_1D-1; ++i) {
    if (Preds[i] != 0)
      b[i] = c[i] + 1;
    else
      a[i] = i * i;
  }
}
```

LLVM will optimize the IR to a single store by a phi instruction:

   ```llvm
      %1 = load ptr, ptr @A, align 64
      %2 = load ptr, ptr @b, align 64
      ...
    for.inc:
      %.sink = phi ptr [ %1, %if.then ], [ %2, %if.else ]
      %add.sink = phi double [ %add, %if.then ], [ %conv8, %if.else ]
%arrayidx7 = getelementptr inbounds double, ptr %.sink, i64 %indvars.iv
      store double %add.sink, ptr %arrayidx7, align 8
   ```

LAA is currently unable to analyze such IR, since ScalarEvolution will
return a SCEVUnknown for the forked pointer operand of the store.

This patch adds initial optional support for analyzing both
possibilities for the pointer and allowing LAA to generate runtime
checks for the bounds if required, refers to D108699, but here address
the phi node.

Fixes llvm#64888

Reviewed By: huntergr-arm, fhahn
Differential Revision: https://reviews.llvm.org/D158965
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants