clangir icon indicating copy to clipboard operation
clangir copied to clipboard

[CIR][Lowering] Add the concept of simple lowering and use it to implement FP intrincis

Open philnik777 opened this issue 1 year ago • 2 comments

philnik777 avatar Jan 31 '24 13:01 philnik777

This is an awesome feature! This is not a blocker for the PR, just some ideas for further development. This feature may be generalized in the future to declare lowering rules more flexibly. Consider the cir.bit family of operations, which has an LLVMIR lowering scheme like the following:

%1 = cir.bit.clz(%0 : !u32i) : !s32i
// %1 = llvm.call_intrinsic "llvm.ctlz.i32"(%0 : i32) : i32 -> i32

%2 = cir.bit.ctz(%0 : !u32i) : !s32i
// %2 = llvm.call_intrinsic "llvm.cttz.i32"(%0 : i32) : i32 -> i32

%3 = cir.bit.popcount(%0 : !u32i) : !s32i
// %3 = llvm.call_intrinsic "llvm.ctpop.i32"(%0 : i32) : i32 -> i32

Basically all cir.bit.* operation will be lowered to an llvm.call_intrinsic operation whose intrin parameter is determined by the CIR operation type.

Stuff in this PR cannot be used to achieve the lowering scheme above because we cannot specify the parameters to the LLVMIR operations. It would be nice if this is added in the future; it would be very useful for lowering compiler intrinsics as shown above.

Lancern avatar Mar 06 '24 13:03 Lancern

Good point @Lancern, we can definitely extend the mechanism later to also support things that get lowered to llvm.call_intrinsic.

bcardosolopes avatar Mar 06 '24 20:03 bcardosolopes

Any progress on this PR?

Lancern avatar Aug 23 '24 04:08 Lancern

@philnik777 is probably busy with libc++ work.

@Lancern if you are interested in making progress here, my suggestion:

  1. Cherry-pick his work locally (so as to preserve original authorship) and fix some of the conflicts.
  2. Add your changes on top + previous reviewer feedback.
  3. Take over this PR (assuming that's possible)

I'll then land without squashing so that we can preserve your work and his (assuming we get a final total of 2 commits, his + yours).

bcardosolopes avatar Aug 23 '24 17:08 bcardosolopes

Sorry for dropping this. I completely forgot that I had a patch open here. @Lancern if you want to, feel free to take the patch and just add a Co-authored-by: Nikolas Klauser <[email protected]> in the commit message.

philnik777 avatar Aug 24 '24 09:08 philnik777

Let me work on this PR.

Lancern avatar Aug 24 '24 12:08 Lancern

@philnik777 np, thanks for the clangir contribs so far!

bcardosolopes avatar Aug 26 '24 17:08 bcardosolopes

This is now tracked in https://github.com/llvm/clangir/pull/806, closing

bcardosolopes avatar Aug 26 '24 21:08 bcardosolopes