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

Conversation

jashook
Copy link

@jashook jashook commented Feb 6, 2017

We will unconditionally assign blkNode->gtBlkOpKind to BlkOpKindHelper in lsraarm64.cpp. Which makes the assert seemingly unnecessary.

@CarolEidt @briansull ptal
/cc @dotnet/arm64-contrib @dotnet/jit-contrib

We will unconditionally assign blkNode->gtBlkOpKind to BlkOpKindHelper in lsraarm64.cpp.
@jashook
Copy link
Author

jashook commented Feb 6, 2017

@dotnet-bot test Windows_NT Arm64 Debug

@CarolEidt
Copy link

The fundamental problem is that Lowering::LowerBlockStore() isn't being called in the arm64 case. The fact that the gtBlkOpKind is being set in the TreeNodeInfoInitBlockStore methods is also a (less significant) bug. The proper fix is to add a call to LowerBlockStore() before the call to TreeNodeInfoInitBlockStore() as is done in lsraxarch.cpp. For now it doesn't really matter, but longer term we will want to ensure consistency (e.g. when we add support for unrolling), and we want to make sure that these kinds of decisions are consistently made in Lowering.
BTW - thanks @jashook for picking this up so quickly!

@jashook
Copy link
Author

jashook commented Feb 7, 2017

@CarolEidt thank you for the insight. I have opened PR #9385 to correctly address this based on your feedback. I have some testing still outstanding; however, it is good so far.

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