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][ThroughMLIR] Lower CIR IV load with SCF IV move operation #729

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

ShivaChen
Copy link
Contributor

Previously, when lowering induction variable in forOp, we removed the IV load and replaced the users with SCF.IV.

The CIR IV users might still CIR operations during lowering forOp. It caused the issue that CIR operation contained SCF.IV as operand which is MLIR integer type instead CIR type.

This comment lower
CIR load IV_ADDR
with
ARITH addi SCF.IV, 0

So SCF.IV can be propagated by OpAdaptor when lowering individual IV users. This simplifies the lowering and fixes the issue. The redundant arith.addi can be removed by later MLIR passes.

Previously, when lowering induction variable in forOp, we removed the
IV load and replaced the users with SCF.IV.

The CIR IV users might still CIR operations during lowering forOp.
It caused the issue that CIR operation contained SCF.IV as operand
which is MLIR integer type instead CIR type.

This comment lower
    CIR load IV_ADDR
with
    ARITH addi SCF.IV, 0

So SCF.IV can be propagated by OpAdaptor when lowering individual
IV users. This simplifies the lowering and fixes the issue.
The redundant arith.addi can be removed by later MLIR passes.
Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

LGTM

@bcardosolopes bcardosolopes merged commit dd9353b into llvm:main Jul 10, 2024
7 checks passed
Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request Oct 2, 2024
…#729)

Previously, when lowering induction variable in forOp, we removed the IV
load and replaced the users with SCF.IV.

The CIR IV users might still CIR operations during lowering forOp. It
caused the issue that CIR operation contained SCF.IV as operand which is
MLIR integer type instead CIR type.

This comment lower
    CIR load IV_ADDR
with
    ARITH addi SCF.IV, 0

So SCF.IV can be propagated by OpAdaptor when lowering individual IV
users. This simplifies the lowering and fixes the issue. The redundant
arith.addi can be removed by later MLIR passes.
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
…#729)

Previously, when lowering induction variable in forOp, we removed the IV
load and replaced the users with SCF.IV.

The CIR IV users might still CIR operations during lowering forOp. It
caused the issue that CIR operation contained SCF.IV as operand which is
MLIR integer type instead CIR type.

This comment lower
    CIR load IV_ADDR
with
    ARITH addi SCF.IV, 0

So SCF.IV can be propagated by OpAdaptor when lowering individual IV
users. This simplifies the lowering and fixes the issue. The redundant
arith.addi can be removed by later MLIR passes.
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
…#729)

Previously, when lowering induction variable in forOp, we removed the IV
load and replaced the users with SCF.IV.

The CIR IV users might still CIR operations during lowering forOp. It
caused the issue that CIR operation contained SCF.IV as operand which is
MLIR integer type instead CIR type.

This comment lower
    CIR load IV_ADDR
with
    ARITH addi SCF.IV, 0

So SCF.IV can be propagated by OpAdaptor when lowering individual IV
users. This simplifies the lowering and fixes the issue. The redundant
arith.addi can be removed by later MLIR passes.
keryell pushed a commit to keryell/clangir that referenced this pull request Oct 19, 2024
…#729)

Previously, when lowering induction variable in forOp, we removed the IV
load and replaced the users with SCF.IV.

The CIR IV users might still CIR operations during lowering forOp. It
caused the issue that CIR operation contained SCF.IV as operand which is
MLIR integer type instead CIR type.

This comment lower
    CIR load IV_ADDR
with
    ARITH addi SCF.IV, 0

So SCF.IV can be propagated by OpAdaptor when lowering individual IV
users. This simplifies the lowering and fixes the issue. The redundant
arith.addi can be removed by later MLIR passes.
lanza pushed a commit that referenced this pull request Nov 5, 2024
Previously, when lowering induction variable in forOp, we removed the IV
load and replaced the users with SCF.IV.

The CIR IV users might still CIR operations during lowering forOp. It
caused the issue that CIR operation contained SCF.IV as operand which is
MLIR integer type instead CIR type.

This comment lower
    CIR load IV_ADDR
with
    ARITH addi SCF.IV, 0

So SCF.IV can be propagated by OpAdaptor when lowering individual IV
users. This simplifies the lowering and fixes the issue. The redundant
arith.addi can be removed by later MLIR passes.
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