[CIR][ThroughMLIR] Support lowering ForOp to scf

[ShivaChen]

This commit introduces CIRForOpLowering for lowering to scf.

The initial commit only support increment loop with lt or le comparison.

[bcardosolopes]

Nice, thanks! How does this relate to the other loop canonicalization PR?

I'd be good if you can split the implementation into another file, so we can mitigate this early on, differently from the current state of LowerToLLVM.cpp.

[ShivaChen]

Nice, thanks! How does this relate to the other loop canonicalization PR?

WIth the other PR which guarantee IV in the LHS of loop comparison, we could use RHS to search the loop boundary and hoisting the boundary operations out of the loop would allow the conversion using the value as input operand for SCF loop.

I'd be good if you can split the implementation into another file, so we can mitigate this early on, differently from the current state of LowerToLLVM.cpp.

It seems to be a good idea. I will work in this direction. Thanks!

[ShivaChen]

I'd be good if you can split the implementation into another file, so we can mitigate this early on, differently from the current state of LowerToLLVM.cpp.

It seems to be a good idea. I will work in this direction. Thanks!

I tried to implement a pass before ConvertCIRToMLIRPass which only convert CIR for loop to SCF. Because SCF for loop required the inputs are builtin integer types(index or i32/i64), the operand checking will report error. It seems CIRForOpLowering might live in ConvertCIRToMLIRPass to make sure the operations before loops have been converted to MLIR. Would you like me to try other way to split the implementation or live CIRForOpLowering as it is?

[bcardosolopes]

I tried to implement a pass before ConvertCIRToMLIRPass which only convert CIR for loop to SCF. Because SCF for loop required the inputs are builtin integer types(index or i32/i64), the operand checking will report error. It seems CIRForOpLowering might live in ConvertCIRToMLIRPass to make sure the operations before loops have been converted to MLIR. Would you like me to try other way to split the implementation or live CIRForOpLowering as it is?

Sorry for the confusion, my suggestion is only about the mechanical part of splitting the implementation in more files, no need to change the overall approach of the PR.

[ShivaChen]

I tried to implement a pass before ConvertCIRToMLIRPass which only convert CIR for loop to SCF. Because SCF for loop required the inputs are builtin integer types(index or i32/i64), the operand checking will report error. It seems CIRForOpLowering might live in ConvertCIRToMLIRPass to make sure the operations before loops have been converted to MLIR. Would you like me to try other way to split the implementation or live CIRForOpLowering as it is?

Sorry for the confusion, my suggestion is only about the mechanical part of splitting the implementation in more files, no need to change the overall approach of the PR.

I will put the implementation in a separate file. Thanks for the clarification.

[bcardosolopes]

LGTM with one note for future work.

[bcardosolopes]

Can you add a comment with the original C/C++ code you are building or write the test in a way that is testable all the way from the source file? As we change CIR, I'm afraid these tests are going to get hard to update. We already have that problem elsewhere, but probably best not to increase them. Similar for your other PR. No need to tackle right now, a future PR sounds good.

[ShivaChen]

Thanks for letting me know. I will create a PR to update for.cir soon.

[GaoXiangYa]

Hellow, I completed the CIRConditionLowering and CIRWhileLowering, do we need to put these codes in the LoweringCIRLoopTO SCF? @ShivaChen @bcardosolopes

[ShivaChen]

Hellow, I completed the CIRConditionLowering and CIRWhileLowering, do we need to put these codes in the LoweringCIRLoopTO SCF? @ShivaChen @bcardosolopes

It would be great addition. Thank you!