Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Using struct for VectorPacket in PacketTracer benchmark #19662

Merged
merged 1 commit into from
Aug 29, 2018
Merged

Using struct for VectorPacket in PacketTracer benchmark #19662

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/src/JIT/Performance/CodeQuality/HWIntrinsic/X86/PacketTracer/VectorPacket.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
using System.Runtime.CompilerServices;
using System;

internal class VectorPacket256
internal struct VectorPacket256
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are using in, this should probably be readonly struct to ensure you don't incur any hidden copies.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to update VectorPacket256 fields in the loop https://github.com/dotnet/coreclr/pull/19662/files#diff-68935896f95ba1d13b35f248c35f1e63R173

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, seems we can optimize here, will try.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then you may be incurring hidden copies when passing the value as in. I would recommend checking this with the ErrorProne.NET analyzer

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then you may be incurring hidden copies when passing the value as in.

Probably not, that loop just updates a local struct. Will try to eliminate the field update.

Copy link
Author

@fiigii fiigii Aug 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to eliminate the field update of local structs via updating local vectors then new the readonly VectorPacket by the local vectors.
This change makes a little bit slower (0.75s vs 0.74s) and a bit code size regression due to callee-saved SIMD register prolog.

So, I think mutable struct is okay for this program.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, I have updated this PR to use pass-by-value.

{
public Vector256<float> Xs;
public Vector256<float> Ys;
Expand Down