Support for string desc in C backend #8056
Conversation
| type = string_descriptor; | ||
| } else { | ||
| type = character_type; | ||
| } |
There was a problem hiding this comment.
@assem2002 is this a correct change?
There was a problem hiding this comment.
That seems a bit awkward, I think BindC in main uses llvm type = character_type and the physicalType should've always been PointerString.
If that unblocks some needed work, You can go for it.
There was a problem hiding this comment.
@swamishiju does this unblock you in some way? Is there some Fortran code that shows what this fixes?
There was a problem hiding this comment.
This was a fix for @ccall in LP
@ccall
def f_str_i32(x:str) -> i32 :
passdeclare i32 @f_str_i32(i8**)
...
code generation error: asr_to_llvm: module failed verification. Error:
Call parameter type does not match function signature!
%casted_string_ptr_to_desc = alloca %string_descriptor, align 8
i8** %32 = call i32 @f_str_i32(%string_descriptor* %casted_string_ptr_to_desc)Initially it generated an llvm function with i8** for BindC functions with string arguments.
I had two options, create another cast for this case to convert string descriptor to pointer string, or change the argument based on physical type. After that the llvm test works but CPython and C tests fail.
The C-failure is unrelated its just physical cast not being implemented.
The CPython failure is some size initialization issue i need to look into
There was a problem hiding this comment.
It was for this PR but it needs some more changes lcompilers/lpython#2864
|
May you mention the issue this PR is trying to fix @swamishiju |
| @@ -688,7 +687,11 @@ namespace LCompilers { | |||
| ASR::String_t* v_type = ASR::down_cast<ASR::String_t>(asr_type); | |||
| a_kind = v_type->m_kind; | |||
| if (arg_m_abi == ASR::abiType::BindC) { | |||
| type = character_type; | |||
| if (v_type->m_physical_type == ASR::string_physical_typeType::DescriptorString) { | |||
| type = string_descriptor; | |||
There was a problem hiding this comment.
Set type according to physical type
| @@ -10372,7 +10372,7 @@ class ASRToLLVMVisitor : public ASR::BaseVisitor<ASRToLLVMVisitor> | |||
| tmp = llvm_utils->CreateLoad(tmp); | |||
| } | |||
| } else { | |||
| if (!arg->m_value_attr && !ASR::is_a<ASR::String_t>(*arg_type)) { | |||
| if (!arg->m_value_attr && !ASR::is_a<ASR::String_t>(*ASRUtils::extract_type(arg_type))) { | |||
There was a problem hiding this comment.
Since string descriptor is usually inside allocatable this would not normally be true for it
| type = string_descriptor; | ||
| } else { | ||
| type = character_type; | ||
| } |
There was a problem hiding this comment.
This was a fix for @ccall in LP
@ccall
def f_str_i32(x:str) -> i32 :
passdeclare i32 @f_str_i32(i8**)
...
code generation error: asr_to_llvm: module failed verification. Error:
Call parameter type does not match function signature!
%casted_string_ptr_to_desc = alloca %string_descriptor, align 8
i8** %32 = call i32 @f_str_i32(%string_descriptor* %casted_string_ptr_to_desc)Initially it generated an llvm function with i8** for BindC functions with string arguments.
I had two options, create another cast for this case to convert string descriptor to pointer string, or change the argument based on physical type. After that the llvm test works but CPython and C tests fail.
The C-failure is unrelated its just physical cast not being implemented.
The CPython failure is some size initialization issue i need to look into
| type = string_descriptor; | ||
| } else { | ||
| type = character_type; | ||
| } |
There was a problem hiding this comment.
It was for this PR but it needs some more changes lcompilers/lpython#2864
|
@swamishiju go ahead and fix the LPython PR first to see that it works. Then we can consider merging this. Even better would be to submit a test case that breaks in LFortran and this PR fixes it. |
|
conflicts need to be resolved |
|
I need to look into this again, I have a feeling these changes might not be required - but ye I need to look into it. |
No description provided.