Skip to content

Reverting dead unallignedCode paths - #1845

Merged
Anipik merged 3 commits into
dotnet:masterfrom
Anipik:revertAlignedArray
Dec 10, 2018
Merged

Anipik merged 3 commits into
dotnet:masterfrom
Anipik:revertAlignedArray

Conversation

@Anipik

@Anipik Anipik commented Dec 6, 2018

Copy link
Copy Markdown
Contributor

Reverting unaligned CodePaths were added in #1218 and #1274

Related to #1838

@Anipik

Anipik commented Dec 8, 2018

Copy link
Copy Markdown
Contributor Author

@Ivanidzo4ka @eerhardt @TomFinley @tannergooding can you please review this one ?

Comment thread test/Microsoft.ML.TimeSeries.Tests/TimeSeriesEstimatorTests.cs Outdated
Comment thread test/Microsoft.ML.CpuMath.PerformanceTests/AvxPerformanceTests.cs Outdated

@eerhardt eerhardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

Comment thread src/Microsoft.ML.CpuMath/AvxIntrinsics.cs Outdated
Comment thread src/Microsoft.ML.CpuMath/AvxIntrinsics.cs
Comment thread test/Microsoft.ML.CpuMath.PerformanceTests/PerformanceTests.cs Outdated

@tannergooding tannergooding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. A couple of nits on places where we could still be generating better code

@eerhardt
eerhardt requested a review from codemzs December 10, 2018 19:56
Comment thread src/Microsoft.ML.CpuMath/CpuMathUtils.netstandard.cs Outdated
public static void MatrixTimesSource(bool transpose, AlignedArray matrix, AlignedArray source, AlignedArray destination, int stride) => SseUtils.MatTimesSrc(transpose, matrix, source, destination, stride);
private static bool Compat(AlignedArray a)
{
Contracts.AssertValue(a);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Moving code like this is really hard to review. Did you make any changes to this code, or is it a straight copy from Sse.cs to this file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it is straight copy from sse.cs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The only addition is the function GetVectorAlignment

@Anipik

Anipik commented Dec 10, 2018

Copy link
Copy Markdown
Contributor Author

@eerhardt can we merge this one ?

@eerhardt

Copy link
Copy Markdown
Member

Since we are editing Time Series code, I think it would be good for @codemzs to sign off on the changes to it.

@codemzs codemzs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

@Anipik
Anipik merged commit 287bc3e into dotnet:master Dec 10, 2018
@Anipik
Anipik deleted the revertAlignedArray branch December 10, 2018 23:23
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants