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

Commit

Permalink
Fix Broadcast
Browse files Browse the repository at this point in the history
  • Loading branch information
@CarolEidt
CarolEidt committed Mar 21, 2019
1 parent 0ae8f19 commit f1ae7f8
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/jit/lowerxarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2671,7 +2671,10 @@ bool Lowering::IsContainableHWIntrinsicOp(GenTreeHWIntrinsic* containingNode, Ge
case NI_AVX2_BroadcastScalarToVector128:
case NI_AVX2_BroadcastScalarToVector256:
{
return (node->TypeGet() != TYP_SIMD16);
// The memory form of this already takes a pointer, and cannot be further contained.
// The containable form is the one that takes a SIMD value, that may be in memory.
supportsGeneralLoads = (node->TypeGet() == TYP_SIMD16);

This comment has been minimized.

Sign in to view

Copy link
@tannergooding

tannergooding Mar 21, 2019

Member

Mostly just talking myself through this.

This is correct and support things like stack spills or field loads since we have 16-bytes and only read the lower element.

This comment has been minimized.

Sign in to view

Copy link
@CarolEidt

CarolEidt Mar 21, 2019

Author

Yes, though I'm not happy about the type inconsistency between the different forms of this intrinsic. I think we'd be better off modeling this, in the JIT, as an intrinsic that always takes a value of the element type.
I think it would be OK to leave the API as-is (though IMO it's a bit weird as it seems to imply that the instruction actually reads a full vector register in the non-mem case), and to make the transformation in the JIT when the intrinsic is imported.
break;
}

case NI_SSE_ConvertScalarToVector128Single:
Expand Down

0 comments on commit f1ae7f8

Please sign in to comment.