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

Create hwintrinsic.cpp#16097

Merged
tannergooding merged 1 commit into
dotnet:masterfrom
sdmaclea:PR-Create-hwintrinsic.cpp
Jan 31, 2018
Merged

Create hwintrinsic.cpp#16097
tannergooding merged 1 commit into
dotnet:masterfrom
sdmaclea:PR-Create-hwintrinsic.cpp

Conversation

@sdmaclea
Copy link
Copy Markdown

@sdmaclea sdmaclea commented Jan 30, 2018

Start sharing hwintrinsic code.

@tannergooding @CarolEidt @fiigii PTAL

sdmaclea
sdmaclea commented Jan 30, 2018
View reviewed changes
Comment thread src/jit/hwintrinsicArm64.cpp Outdated
}
simdType = getSIMDTypeForSize(simdSizeBytes);
}

Copy link
Copy Markdown
Author

@sdmaclea sdmaclea Jan 30, 2018

Choose a reason for hiding this comment

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

Merge error...

@tannergooding
Copy link
Copy Markdown
Member

tannergooding commented Jan 30, 2018 •
edited
Loading

Other than the merge error, this LGTM.

CarolEidt
CarolEidt approved these changes Jan 30, 2018
View reviewed changes
Copy link
Copy Markdown

@CarolEidt CarolEidt left a comment

Choose a reason for hiding this comment

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

LGTM with one suggestion for a comment fix

Comment thread src/jit/hwintrinsicArm64.cpp Outdated

if (simdBaseType == TYP_UNKNOWN)
{
// TODO-FIXME Add CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED
Copy link
Copy Markdown

@CarolEidt CarolEidt Jan 30, 2018

Choose a reason for hiding this comment

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

Nit: Please follow the conventions for TODO comments (https://github.com/dotnet/coreclr/blob/master/Documentation/coding-guidelines/clr-jit-coding-conventions.md#715-todo) - e.g. I think this should probably be TODO-Bug

Copy link
Copy Markdown
Author

@sdmaclea sdmaclea Jan 30, 2018

Choose a reason for hiding this comment

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

👍 I'll fix this in #16085 (where this stray bit of code came from)

Copy link
Copy Markdown

@CarolEidt CarolEidt Jan 30, 2018

Choose a reason for hiding this comment

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

Sounds good. Thanks

@fiigii
Copy link
Copy Markdown

fiigii commented Jan 30, 2018

Thanks! Please run some hwintrinsic tests before merge.

@sdmaclea sdmaclea force-pushed the PR-Create-hwintrinsic.cpp branch from cdb3850 to 31f7fe5 Compare January 30, 2018 19:44
@sdmaclea
Copy link
Copy Markdown
Author

sdmaclea commented Jan 30, 2018

Fixed merge issue.

@tannergooding @fiigii Can you trigger X86 HW intrinsic tests. I have run on Arm64 already.

@tannergooding
Copy link
Copy Markdown
Member

tannergooding commented Jan 30, 2018

test Windows_NT x64 Checked jitincompletehwintrinsic
test Windows_NT x64 Checked jitx86hwintrinsicnoavx
test Windows_NT x64 Checked jitx86hwintrinsicnoavx2
test Windows_NT x64 Checked jitx86hwintrinsicnosimd
test Windows_NT x64 Checked jitnox86hwintrinsic

test Windows_NT x86 Checked jitincompletehwintrinsic
test Windows_NT x86 Checked jitx86hwintrinsicnoavx
test Windows_NT x86 Checked jitx86hwintrinsicnoavx2
test Windows_NT x86 Checked jitx86hwintrinsicnosimd
test Windows_NT x86 Checked jitnox86hwintrinsic

@tannergooding
Copy link
Copy Markdown
Member

tannergooding commented Jan 30, 2018

test Ubuntu x64 Checked jitincompletehwintrinsic
test Ubuntu x64 Checked jitx86hwintrinsicnoavx
test Ubuntu x64 Checked jitx86hwintrinsicnoavx2
test Ubuntu x64 Checked jitx86hwintrinsicnosimd
test Ubuntu x64 Checked jitnox86hwintrinsic

@sdmaclea sdmaclea mentioned this pull request Jan 30, 2018
Merged
@sdmaclea
Copy link
Copy Markdown
Author

sdmaclea commented Jan 30, 2018

test Tizen armel Cross Checked Innerloop Build and Test

@sdmaclea
Copy link
Copy Markdown
Author

sdmaclea commented Jan 31, 2018

Ubuntu x64 Checked Build and Test (Jit - EnableIncompleteISAClass=1)
Failure unrelated to this change and can be ignored.
FAILED - GC/Scenarios/LeakWheel/leakwheel/leakwheel.sh

@sdmaclea
Copy link
Copy Markdown
Author

sdmaclea commented Jan 31, 2018

Please merge

@tannergooding tannergooding merged commit 51a8ec0 into dotnet:master Jan 31, 2018
@sdmaclea sdmaclea deleted the PR-Create-hwintrinsic.cpp branch May 24, 2018 19:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@CarolEidt CarolEidt CarolEidt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-CodeGen

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sdmaclea @tannergooding @fiigii @CarolEidt @jkotas