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

[CIR][CIRGen][Builtin][Neon] Lower vld1_dup and vld1q_dup #936

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

ghehg
Copy link
Collaborator

@ghehg ghehg commented Oct 3, 2024

No description provided.

clang/include/clang/CIR/Dialect/IR/CIROps.td Outdated Show resolved Hide resolved
clang/test/CIR/CodeGen/neon-tmp.c Outdated Show resolved Hide resolved
@bcardosolopes
Copy link
Member

I know it's a draft but felt like ready to review!

@bcardosolopes
Copy link
Member

Once you change the PR it's ready to publish (imo)

@ghehg ghehg force-pushed the main branch 2 times, most recently from 42f5697 to 9ed1679 Compare October 10, 2024 14:31
bcardosolopes pushed a commit that referenced this pull request Oct 11, 2024
…nOp (#959)

They should use PoisonOp (which becomes PoisonValue in LLVMIR) as it is
the OG's choice.
Proof:
We generate VecCreateOp [here
](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp#L1975)
And it's OG counterpart is
[here](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/clang/lib/CodeGen/CGExprScalar.cpp#L2096)
OG uses PoisonValue. 
As to VecSplatOp, OG unconditionally [chooses PoisonValue
](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/llvm/lib/IR/IRBuilder.cpp#L1204)

A even more solid proof for this case is that 
when we use OG to generate code for our test case I changed in this PR ,
its always using poison instead of undef as far as VecSplat and
VecCreate is concerned. The [OG generated code for vectype-ext.cpp
](https://godbolt.org/z/eqx1rns86) here.
The [OG generated code for vectype.cpp
](https://godbolt.org/z/frMjbKGeT) here.

For reference, generated CIR for the test case vectype-ext.cpp is
[here](https://godbolt.org/z/frMjbKGeT)

This is to unblock #936 to help it
set on the right path.

Note: There might be other CIR vec ops that need to choose Poison to be
consistent with OG, but I'd limit the scope of this PR, and wait to see
issue pop up in the future.
@ghehg ghehg force-pushed the main branch 2 times, most recently from 63d44e5 to c40bb23 Compare October 13, 2024 02:34
@ghehg ghehg marked this pull request as ready for review October 13, 2024 20:33
@ghehg ghehg requested a review from lanza as a code owner October 13, 2024 20:33
@bcardosolopes bcardosolopes merged commit 1fc3d42 into llvm:main Oct 14, 2024
8 checks passed
keryell pushed a commit to keryell/clangir that referenced this pull request Oct 19, 2024
…nOp (llvm#959)

They should use PoisonOp (which becomes PoisonValue in LLVMIR) as it is
the OG's choice.
Proof:
We generate VecCreateOp [here
](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp#L1975)
And it's OG counterpart is
[here](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/clang/lib/CodeGen/CGExprScalar.cpp#L2096)
OG uses PoisonValue. 
As to VecSplatOp, OG unconditionally [chooses PoisonValue
](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/llvm/lib/IR/IRBuilder.cpp#L1204)

A even more solid proof for this case is that 
when we use OG to generate code for our test case I changed in this PR ,
its always using poison instead of undef as far as VecSplat and
VecCreate is concerned. The [OG generated code for vectype-ext.cpp
](https://godbolt.org/z/eqx1rns86) here.
The [OG generated code for vectype.cpp
](https://godbolt.org/z/frMjbKGeT) here.

For reference, generated CIR for the test case vectype-ext.cpp is
[here](https://godbolt.org/z/frMjbKGeT)

This is to unblock llvm#936 to help it
set on the right path.

Note: There might be other CIR vec ops that need to choose Poison to be
consistent with OG, but I'd limit the scope of this PR, and wait to see
issue pop up in the future.
keryell pushed a commit to keryell/clangir that referenced this pull request Oct 19, 2024
lanza pushed a commit that referenced this pull request Nov 5, 2024
…nOp (#959)

They should use PoisonOp (which becomes PoisonValue in LLVMIR) as it is
the OG's choice.
Proof:
We generate VecCreateOp [here
](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp#L1975)
And it's OG counterpart is
[here](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/clang/lib/CodeGen/CGExprScalar.cpp#L2096)
OG uses PoisonValue. 
As to VecSplatOp, OG unconditionally [chooses PoisonValue
](https://github.com/llvm/clangir/blob/2ca12fe5ec3a1e7279256f069010be2d68200585/llvm/lib/IR/IRBuilder.cpp#L1204)

A even more solid proof for this case is that 
when we use OG to generate code for our test case I changed in this PR ,
its always using poison instead of undef as far as VecSplat and
VecCreate is concerned. The [OG generated code for vectype-ext.cpp
](https://godbolt.org/z/eqx1rns86) here.
The [OG generated code for vectype.cpp
](https://godbolt.org/z/frMjbKGeT) here.

For reference, generated CIR for the test case vectype-ext.cpp is
[here](https://godbolt.org/z/frMjbKGeT)

This is to unblock #936 to help it
set on the right path.

Note: There might be other CIR vec ops that need to choose Poison to be
consistent with OG, but I'd limit the scope of this PR, and wait to see
issue pop up in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants