-
-
Notifications
You must be signed in to change notification settings - Fork 643
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
Refactored arraysetlengthT
and arraysetlengthiT
into a Single arraysetlengthT
Template
#21151
base: master
Are you sure you want to change the base?
Conversation
….c fixing xtest46.d assertions
Thanks for your pull request and interest in making D better, @shivangshukla7020! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#21151" |
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.
Overall this is looking good. I made some small suggestions.
} | ||
|
||
version (D_ProfileGC) | ||
size_t oldsize = arr.length * sizeelem; | ||
bool isshared = is(T == shared T); |
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.
You can optimise this a little bit. Now a new template instance would be emitted for each array type like A[]
, const A[]
etc, but you only need the unqualified type (UnqT
). So draw inspiration from here [1] and unqualify the type in the compiler and pass isshared
as an argument to reduce the number of generated type instances.
[1]
dmd/compiler/src/dmd/expressionsem.d
Lines 4852 to 4859 in 8bce4f5
auto tiargs = new Objects(); | |
/* | |
* Remove `inout`, `const`, `immutable` and `shared` to reduce the | |
* number of generated `_d_newitemT` instances. | |
*/ | |
auto t = ne.type.nextOf.unqualify(MODFlags.wild | MODFlags.const_ | | |
MODFlags.immutable_ | MODFlags.shared_); | |
tiargs.push(t); |
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.
I’ll update it to follow that approach and avoid unnecessary template instantiations .
Co-authored-by: Teodor Dutu <[email protected]>
Co-authored-by: Teodor Dutu <[email protected]>
Co-authored-by: Teodor Dutu <[email protected]>
Co-authored-by: Teodor Dutu <[email protected]>
e8265b8
to
d1bc7e6
Compare
@@ -3764,14 +3764,17 @@ extern (C++) final class SliceExp : UnaExp | |||
*/ | |||
extern (C++) final class ArrayLengthExp : UnaExp | |||
{ | |||
Expression lowering; // Field to store the lowered expression |
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.
You don't need this field. expressionsem.d
creates a LoweredAssignExp
which already contains the lowering. No need to duplicate it here.
Co-authored-by: Teodor Dutu <[email protected]>
… hello-profile.d trace output for x86 compatibility + full construction of hook in expressionsem.d
Changes Introduced
This PR merges
arraysetlengthT
andarraysetlengthiT
into a single template, improving compile-time specialization and reducing redundancy. The key updates include:arraysetlengthT
andarraysetlengthiT
into a single template, distinguishing them based on initialization behavior.arraysetlengthT
) and non-zero-initialized (arraysetlengthiT
) cases.emplace
for non-trivial types instead ofmemcpy
to ensure correct postblit behavior.gc_expandArrayUsed
for in-place array expansion where applicable.D_ProfileGC
support via_d_HookTraceImpl
.This refactoring improves performance by removing
TypeInfo
lookups and enabling better compile-time specialization.