-
Notifications
You must be signed in to change notification settings - Fork 98
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] Fix FuncOp for functions with pointer arguments. #684
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Do we need to consider ABI problems here? @SchrodingerZhu pointed out that clangir/mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp Lines 311 to 326 in 0c86870
If ABI should be concerned we may revert this PR. |
Do you mean that we should not simply convert cir.ptr to memref? If such a conversion has issues, you can revert this PR at any time. |
However, I need to clarify that this PR only addresses the issue where cir.ptr or cir.vec (and similar types) cannot be correctly converted when used as function parameters. |
The semantics are equivalent, I'm just not sure if it's OK to break C/C++ ABI guarantees on the MLIR lowering path. If ABI is not the problem this lowering is good. |
…lvm#684) This PR is to fix the issue llvm#658 . Now we can get the correct result using the following command. ``` echo "void test(int *){}" | ./build/Debug/bin/clang -cc1 -triple x86_64-unknown-linux-gnu -fclangir -fno-clangir-direct-lowering -emit-mlir -o - ``` result: ``` module attributes {cir.lang = #cir.lang<c>, cir.sob = #cir.signed_overflow_behavior<undefined>, cir.triple = "x86_64-unknown-linux-gnu", dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<!llvm.ptr, dense<64> : vector<4xi64>>, #dlti.dl_entry<i1, dense<8> : vector<2xi64>>, #dlti.dl_entry<f16, dense<16> : vector<2xi64>>, #dlti.dl_entry<i32, dense<32> : vector<2xi64>>, #dlti.dl_entry<i8, dense<8> : vector<2xi64>>, #dlti.dl_entry<i16, dense<16> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr<272>, dense<64> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<271>, dense<32> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<270>, dense<32> : vector<4xi64>>, #dlti.dl_entry<f128, dense<128> : vector<2xi64>>, #dlti.dl_entry<f64, dense<64> : vector<2xi64>>, #dlti.dl_entry<f80, dense<128> : vector<2xi64>>, #dlti.dl_entry<i128, dense<128> : vector<2xi64>>, #dlti.dl_entry<i64, dense<64> : vector<2xi64>>, #dlti.dl_entry<"dlti.stack_alignment", 128 : i64>, #dlti.dl_entry<"dlti.endianness", "little">>, llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"} { func.func @test(%arg0: memref<i32> loc(fused[#loc3, #loc4])) { %alloca = memref.alloca() {alignment = 8 : i64} : memref<memref<i32>> loc(#loc7) memref.store %arg0, %alloca[] : memref<memref<i32>> loc(#loc5) return loc(#loc2) } loc(#loc6) } loc(#loc) ``` And the test/CIR/Lowering/ThroughMLIR/dot.cir now passes the test, so I have removed the XFAIL flag.
…lvm#684) This PR is to fix the issue llvm#658 . Now we can get the correct result using the following command. ``` echo "void test(int *){}" | ./build/Debug/bin/clang -cc1 -triple x86_64-unknown-linux-gnu -fclangir -fno-clangir-direct-lowering -emit-mlir -o - ``` result: ``` module attributes {cir.lang = #cir.lang<c>, cir.sob = #cir.signed_overflow_behavior<undefined>, cir.triple = "x86_64-unknown-linux-gnu", dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<!llvm.ptr, dense<64> : vector<4xi64>>, #dlti.dl_entry<i1, dense<8> : vector<2xi64>>, #dlti.dl_entry<f16, dense<16> : vector<2xi64>>, #dlti.dl_entry<i32, dense<32> : vector<2xi64>>, #dlti.dl_entry<i8, dense<8> : vector<2xi64>>, #dlti.dl_entry<i16, dense<16> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr<272>, dense<64> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<271>, dense<32> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<270>, dense<32> : vector<4xi64>>, #dlti.dl_entry<f128, dense<128> : vector<2xi64>>, #dlti.dl_entry<f64, dense<64> : vector<2xi64>>, #dlti.dl_entry<f80, dense<128> : vector<2xi64>>, #dlti.dl_entry<i128, dense<128> : vector<2xi64>>, #dlti.dl_entry<i64, dense<64> : vector<2xi64>>, #dlti.dl_entry<"dlti.stack_alignment", 128 : i64>, #dlti.dl_entry<"dlti.endianness", "little">>, llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"} { func.func @test(%arg0: memref<i32> loc(fused[#loc3, #loc4])) { %alloca = memref.alloca() {alignment = 8 : i64} : memref<memref<i32>> loc(#loc7) memref.store %arg0, %alloca[] : memref<memref<i32>> loc(#loc5) return loc(#loc2) } loc(#loc6) } loc(#loc) ``` And the test/CIR/Lowering/ThroughMLIR/dot.cir now passes the test, so I have removed the XFAIL flag.
…lvm#684) This PR is to fix the issue llvm#658 . Now we can get the correct result using the following command. ``` echo "void test(int *){}" | ./build/Debug/bin/clang -cc1 -triple x86_64-unknown-linux-gnu -fclangir -fno-clangir-direct-lowering -emit-mlir -o - ``` result: ``` module attributes {cir.lang = #cir.lang<c>, cir.sob = #cir.signed_overflow_behavior<undefined>, cir.triple = "x86_64-unknown-linux-gnu", dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<!llvm.ptr, dense<64> : vector<4xi64>>, #dlti.dl_entry<i1, dense<8> : vector<2xi64>>, #dlti.dl_entry<f16, dense<16> : vector<2xi64>>, #dlti.dl_entry<i32, dense<32> : vector<2xi64>>, #dlti.dl_entry<i8, dense<8> : vector<2xi64>>, #dlti.dl_entry<i16, dense<16> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr<272>, dense<64> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<271>, dense<32> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<270>, dense<32> : vector<4xi64>>, #dlti.dl_entry<f128, dense<128> : vector<2xi64>>, #dlti.dl_entry<f64, dense<64> : vector<2xi64>>, #dlti.dl_entry<f80, dense<128> : vector<2xi64>>, #dlti.dl_entry<i128, dense<128> : vector<2xi64>>, #dlti.dl_entry<i64, dense<64> : vector<2xi64>>, #dlti.dl_entry<"dlti.stack_alignment", 128 : i64>, #dlti.dl_entry<"dlti.endianness", "little">>, llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"} { func.func @test(%arg0: memref<i32> loc(fused[#loc3, #loc4])) { %alloca = memref.alloca() {alignment = 8 : i64} : memref<memref<i32>> loc(#loc7) memref.store %arg0, %alloca[] : memref<memref<i32>> loc(#loc5) return loc(#loc2) } loc(#loc6) } loc(#loc) ``` And the test/CIR/Lowering/ThroughMLIR/dot.cir now passes the test, so I have removed the XFAIL flag.
…lvm#684) This PR is to fix the issue llvm#658 . Now we can get the correct result using the following command. ``` echo "void test(int *){}" | ./build/Debug/bin/clang -cc1 -triple x86_64-unknown-linux-gnu -fclangir -fno-clangir-direct-lowering -emit-mlir -o - ``` result: ``` module attributes {cir.lang = #cir.lang<c>, cir.sob = #cir.signed_overflow_behavior<undefined>, cir.triple = "x86_64-unknown-linux-gnu", dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<!llvm.ptr, dense<64> : vector<4xi64>>, #dlti.dl_entry<i1, dense<8> : vector<2xi64>>, #dlti.dl_entry<f16, dense<16> : vector<2xi64>>, #dlti.dl_entry<i32, dense<32> : vector<2xi64>>, #dlti.dl_entry<i8, dense<8> : vector<2xi64>>, #dlti.dl_entry<i16, dense<16> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr<272>, dense<64> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<271>, dense<32> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<270>, dense<32> : vector<4xi64>>, #dlti.dl_entry<f128, dense<128> : vector<2xi64>>, #dlti.dl_entry<f64, dense<64> : vector<2xi64>>, #dlti.dl_entry<f80, dense<128> : vector<2xi64>>, #dlti.dl_entry<i128, dense<128> : vector<2xi64>>, #dlti.dl_entry<i64, dense<64> : vector<2xi64>>, #dlti.dl_entry<"dlti.stack_alignment", 128 : i64>, #dlti.dl_entry<"dlti.endianness", "little">>, llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"} { func.func @test(%arg0: memref<i32> loc(fused[#loc3, #loc4])) { %alloca = memref.alloca() {alignment = 8 : i64} : memref<memref<i32>> loc(#loc7) memref.store %arg0, %alloca[] : memref<memref<i32>> loc(#loc5) return loc(#loc2) } loc(#loc6) } loc(#loc) ``` And the test/CIR/Lowering/ThroughMLIR/dot.cir now passes the test, so I have removed the XFAIL flag.
…684) This PR is to fix the issue #658 . Now we can get the correct result using the following command. ``` echo "void test(int *){}" | ./build/Debug/bin/clang -cc1 -triple x86_64-unknown-linux-gnu -fclangir -fno-clangir-direct-lowering -emit-mlir -o - ``` result: ``` module attributes {cir.lang = #cir.lang<c>, cir.sob = #cir.signed_overflow_behavior<undefined>, cir.triple = "x86_64-unknown-linux-gnu", dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<!llvm.ptr, dense<64> : vector<4xi64>>, #dlti.dl_entry<i1, dense<8> : vector<2xi64>>, #dlti.dl_entry<f16, dense<16> : vector<2xi64>>, #dlti.dl_entry<i32, dense<32> : vector<2xi64>>, #dlti.dl_entry<i8, dense<8> : vector<2xi64>>, #dlti.dl_entry<i16, dense<16> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr<272>, dense<64> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<271>, dense<32> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<270>, dense<32> : vector<4xi64>>, #dlti.dl_entry<f128, dense<128> : vector<2xi64>>, #dlti.dl_entry<f64, dense<64> : vector<2xi64>>, #dlti.dl_entry<f80, dense<128> : vector<2xi64>>, #dlti.dl_entry<i128, dense<128> : vector<2xi64>>, #dlti.dl_entry<i64, dense<64> : vector<2xi64>>, #dlti.dl_entry<"dlti.stack_alignment", 128 : i64>, #dlti.dl_entry<"dlti.endianness", "little">>, llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"} { func.func @test(%arg0: memref<i32> loc(fused[#loc3, #loc4])) { %alloca = memref.alloca() {alignment = 8 : i64} : memref<memref<i32>> loc(#loc7) memref.store %arg0, %alloca[] : memref<memref<i32>> loc(#loc5) return loc(#loc2) } loc(#loc6) } loc(#loc) ``` And the test/CIR/Lowering/ThroughMLIR/dot.cir now passes the test, so I have removed the XFAIL flag.
This PR is to fix the issue #658 .
Now we can get the correct result using the following command.
result:
And the test/CIR/Lowering/ThroughMLIR/dot.cir now passes the test, so I have removed the XFAIL flag.