M-Labs/nac3
M-Labs/nac3
8
1
Fork 13
Code Issues 96 Pull Requests 6 Releases Wiki Activity

Implement string equality operator using IRRT and optimise LLVM implementation #561

Merged
sb10q merged 9 commits from ramtej/nac3:feature/string-equality into master 2024-12-30 13:02:09 +08:00
Conversation 3 Commits 9 Files Changed 5 +19 -22
2 changed files with 19 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 0b6a9bd89b - Show all commits

2
nac3core/irrt/irrt/string.hpp
View File

@ -8,7 +8,7 @@ SizeT __nac3_str_eq_impl(const char* str1, SizeT len1, const char* str2, SizeT l
if (len1 != len2){
return 0;
}
return (__builtin_strncmp(str1, str2, static_cast<SizeT>(len1)) == 0) ? 1 : 0;
return (__builtin_memcmp(str1, str2, static_cast<SizeT>(len1)) == 0) ? 1 : 0;
sb10q commented 2024-12-13 08:49:12 +08:00
Outdated
Review
Copy Link

Is IRRT really required at this point?

Is IRRT really required at this point?
derppening commented 2024-12-13 10:35:34 +08:00
Outdated
Review
Copy Link

Yes, __builtin_strncmp isn't exposed in LLVM IR so we will need clang to emit the equivalent bitcode for us.

Yes, `__builtin_strncmp` isn't exposed in LLVM IR so we will need `clang` to emit the equivalent bitcode for us.
sb10q commented 2024-12-13 11:38:12 +08:00
Outdated
Review
Copy Link

Isn't clang also just a LLVM user?
How does it do it?

Isn't clang also just a LLVM user? How does it do it?
sb10q commented 2024-12-13 12:37:24 +08:00
Outdated
Review
Copy Link

Also, NAC3 is just using regular Pascal-style strings, isn't it?
If so, then the correct function to use is memcmp. strncmp is intrinsically slower as it adds a check for NUL characters which is unnecessary and may even lead to incorrect behavior.

Also, NAC3 is just using regular Pascal-style strings, isn't it? If so, then the correct function to use is ``memcmp``. ``strncmp`` is intrinsically slower as it adds a check for NUL characters which is unnecessary and may even lead to incorrect behavior.
}
ramtej marked this conversation as resolved Outdated
sb10q commented 2024-12-12 09:25:55 +08:00
Outdated
Review
Copy Link

Isn't that just memcmp but implemented inefficiently?

Isn't that just memcmp but implemented inefficiently?
derppening commented 2024-12-12 10:57:12 +08:00
Outdated
Review
Copy Link

LLVM has an intrinsic __builtin_memcmp for this exact purpose: https://clang.llvm.org/docs/LanguageExtensions.html#string-builtins

LLVM has an intrinsic `__builtin_memcmp` for this exact purpose: https://clang.llvm.org/docs/LanguageExtensions.html#string-builtins
👍 1
derppening commented 2024-12-12 10:58:01 +08:00
Outdated
Review
Copy Link

In fact, LLVM has __builtin_strncmp. If that's the case, the entire IRRT can just be implemented in terms of that intrinsic function.

In fact, LLVM has `__builtin_strncmp`. If that's the case, the entire IRRT can just be implemented in terms of that intrinsic function.
} // namespace

37
nac3core/src/codegen/irrt/string.rs
View File

@ -1,7 +1,4 @@
use inkwell::{
values::{BasicValueEnum, CallSiteValue, IntValue, PointerValue},
AddressSpace,
};
use inkwell::values::{BasicValueEnum, CallSiteValue, IntValue, PointerValue};
use itertools::Either;
use crate::codegen::{CodeGenContext, CodeGenerator};
@ -15,27 +12,27 @@ pub fn call_string_eq<'ctx, G: CodeGenerator + ?Sized>(
str2_ptr: PointerValue<'ctx>,
str2_len: IntValue<'ctx>,
) -> IntValue<'ctx> {
let string_eq_fn = ctx.module.get_function("nac3_str_eq").unwrap_or_else(|| {
let i8_ptr_type = ctx.ctx.i8_type().ptr_type(AddressSpace::default());
let i64_type = ctx.ctx.i64_type();
let i32_type = ctx.ctx.i32_type();
let fn_type = i32_type.fn_type(
&[i8_ptr_type.into(), i64_type.into(), i8_ptr_type.into(), i64_type.into()],
let func = ctx.module.get_function("nac3_str_eq").unwrap_or_else(|| {
ctx.module.add_function(
"nac3_str_eq",
ramtej marked this conversation as resolved Outdated
sb10q commented 2024-12-15 20:54:33 +08:00
Outdated
Review
Copy Link

And when is nac3_str_eq64 called?

And when is ``nac3_str_eq64`` called?
ctx.ctx.i32_type().fn_type(
&[
str1_ptr.get_type().into(),
str1_len.get_type().into(),
str2_ptr.get_type().into(),
str2_len.get_type().into(),
],
false,
);
ctx.module.add_function("nac3_str_eq", fn_type, None)
),
None,
)
});
let result = ctx
.builder
.build_call(
string_eq_fn,
&[
str1_ptr.into(),
ctx.builder.build_int_z_extend(str1_len, ctx.ctx.i64_type(), "").unwrap().into(),
str2_ptr.into(),
ctx.builder.build_int_z_extend(str2_len, ctx.ctx.i64_type(), "").unwrap().into(),
],
"string_eq",
func,
&[str1_ptr.into(), str1_len.into(), str2_ptr.into(), str2_len.into()],
"str_eq_call",
)
.map(CallSiteValue::try_as_basic_value)
.map(|v| v.map_left(BasicValueEnum::into_int_value))