Supporting list of lists, list of tuples and tuples with lists #952
Conversation
|
Excellent! |
|
Thanks @czgdp1807! |
1. Generalised equality check for two values 2. Generalised deepcopy support from source to destination
1. Equality check of two values 2. Deepcopy support for any two llvm values
|
@certik This is ready for review. I have cleaned up the git history so you can review each commit individually. |
| builder->CreateBr(loophead); | ||
|
|
||
| // end | ||
| llvm_utils->start_new_block(loopend); |
There was a problem hiding this comment.
Does this slow anything down in the already existing list (or any other) functionality in master? For example in our current benchmarks for lists? Or is this only used for new features implemented in this PR, but does not affect any feature already in master.
There was a problem hiding this comment.
This code path will only be taken for nesting of types inside lists (which is a new feature not supported earlier). For list[f64], list[i32], etc (i.e., a list with intrinsic element type, the code path in main will be taken). So, theoretically no change in benchmarks should happen.
There was a problem hiding this comment.
When you say "code path", you mean code path at compile time? If so, then this is good.
If you mean "runtime", then that must mean there might be some "if statement" being executed at runtime, and we don't want that.
There was a problem hiding this comment.
When you say "code path", you mean code path at compile time?
Yes code path at compile time.
There was a problem hiding this comment.
Updated the benchmarks for my linux system in #857 (comment). I don't see any change with list05 (the branch of this PR).
There was a problem hiding this comment.
Updated #857 (comment) with results from my macbook, this and main branch give no difference for me.
There was a problem hiding this comment.
Bottomline - This and main branch give same results on both of my systems. So, theoretically and practically from my perspective this PR wouldn't slow down anything for features already existing in main. @certik Do you want to merge it?
|
Yes, for sure. Thank you so much for this. Very exciting. |
No description provided.