[CIR] Add -cir-mlir-scf-prepare to simplify lowering to SCF

[ShivaChen]

This commit introduces SCFPreparePass to 1) Canonicalize IV to LHS of loop comparison For example, transfer cir.cmp(gt, %bound, %IV) to cir.cmp(lt, %IV, %bound). So we could use RHS as boundary and use lt to determine it's an upper bound. 2) Hoist loop invariant operations in condition block out of loop. The condition block may be generated as following which contains the operations produced upper bound. SCF for loop required loop boundary as input operands. So we might need to hoist the boundary operations out of loop.

      cir.for : cond {
        %4 = cir.load %2 : !cir.ptr<!s32i>, !s32i
        %5 = cir.const #cir.int<100> : !s32i       <- upper bound
        %6 = cir.cmp(lt, %4, %5) : !s32i, !s32i
        %7 = cir.cast(int_to_bool, %6 : !s32i), !cir.boo
        cir.condition(%7
       } body {

[bcardosolopes]

Thanks for the PR. Can you share a bit more of what are your plans for using this? I'm assuming this is to make it easier to lower to MLIR, but I'd be great to have more details in the description.

Also, this probably makes more sense elsewhere, lowerprepare is more about additional lowering, not really canonicalization. It could be another pass but an option I'd prefer is to rename MergeCleanups to something a bit more generic and implement it there. It'd also be great if it's easier to integrate later on as part of MLIR generic canonicalization (which we cannot yet use because it's too aggressive). @orbiri any opinions here?

[ShivaChen]

Thanks for the PR. Can you share a bit more of what are your plans for using this? I'm assuming this is to make it easier to lower to MLIR, but I'd be great to have more details in the description.

Sorry for missing more details. I have updated the PR description. Also, this probably makes more sense elsewhere, lowerprepare is more about additional lowering, not really canonicalization. It could be another pass but an option I'd prefer is to rename MergeCleanups to something a bit more generic and implement it there. It'd also be great if it's easier to integrate later on as part of MLIR generic canonicalization (which we cannot yet use because it's too aggressive). @orbiri any opinions here?

Agree that elsewhere would make more sense. I will move to MergeCleanups and perhaps update the pass name when we have agreement.

[ShivaChen]

Thanks for the PR. Can you share a bit more of what are your plans for using this? I'm assuming this is to make it easier to lower to MLIR, but I'd be great to have more details in the description.

Sorry for missing more details. I have updated the PR description.

Also, this probably makes more sense elsewhere, lowerprepare is more about additional lowering, not really canonicalization. It could be another pass but an option I'd prefer is to rename MergeCleanups to something a bit more generic and implement it there. It'd also be great if it's easier to integrate later on as part of MLIR generic canonicalization (which we cannot yet use because it's too aggressive). @orbiri any opinions here?

Agree that elsewhere would make more sense. I will move to MergeCleanups and perhaps update the pass name when we have agreement.

I was separating the canonicalization for SCF loop to this PR because it's more complicated when implementing in LowerCIRToMLIR. Given that we decided to move loop lowering before LowerCIRToMLIR in https://github.com/llvm/clangir/pull/605#issuecomment-2119630720, perhaps another option could be moving the canonicalization to the pass if the canonicalization mere benefit for MLIR path?

[bcardosolopes]

I was separating the canonicalization for SCF loop to this PR because it's more complicated when implementing in LowerCIRToMLIR.

Yea, sounds like the right call to me!

Given that we decided to move loop lowering before LowerCIRToMLIR in #605 (comment), perhaps another option could be moving the canonicalization to the pass if the canonicalization mere benefit for MLIR path?

Not sure I understood, so correct if I'm wrong, but you are suggesting we split this into another pass entirely because it might not be profitable for the non-MLIR lowering path, is that right? If so, sounds good to me too. MergeCleanups will run for both pipelines and you are right, it'd just add compile time for the pure LLVM pipeline - if in the future we see a need, we can enable it for the LLVM pipeline.

[ShivaChen]

I was separating the canonicalization for SCF loop to this PR because it's more complicated when implementing in LowerCIRToMLIR.

Yea, sounds like the right call to me!

Given that we decided to move loop lowering before LowerCIRToMLIR in #605 (comment), perhaps another option could be moving the canonicalization to the pass if the canonicalization mere benefit for MLIR path?

Not sure I understood, so correct if I'm wrong, but you are suggesting we split this into another pass entirely because it might not be profitable for the non-MLIR lowering path, is that right? If so, sounds good to me too. MergeCleanups will run for both pipelines and you are right, it'd just add compile time for the pure LLVM pipeline - if in the future we see a need, we can enable it for the LLVM pipeline.

Yes, I was thinking split it to a new pass. Is there new pass name you would prefer?

[bcardosolopes]

Yes, I was thinking split it to a new pass. Is there new pass name you would prefer?

I'm pretty open, how about -cir-mlir-scf-prepare?

[ShivaChen]

Yes, I was thinking split it to a new pass. Is there new pass name you would prefer?

I'm pretty open, how about -cir-mlir-scf-prepare?

Sounds good to me. Thanks.