Closed Bug 1225908 Opened 4 years ago Closed 4 years ago

Disambiguate SimdTypeToScalarType

Categories

(Core :: JavaScript Engine: JIT, defect)

All
Unspecified
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: jolesen, Assigned: jolesen, Mentored)

Details

Attachments

(3 files)

Ion has two functions called SimdTypeToScalarType with similar purpose:

In IonTypes.h:
  static inline MIRType
  SimdTypeToScalarType(MIRType type)

In MCallOptimize.cpp:
  static Scalar::Type
  SimdTypeToScalarType(SimdTypeDescr::Type type)

We should rename both of these to make their purpose clearer.

Maybe add some documentation too?
Uack, Scalar::Type actually includes Int32x4 and Float32x4.
Maybe we shouldn't refer to vector lanes as 'scalars' to avoid confusion.
Assignee: nobody → jolesen
Rename the function SimdTypeToScalarType to SimdTypeToLaneType. Since we also
have Scalar::Type, it is best to use the term 'lane' to refer to the parts of a
vector.
Attachment #8689122 - Flags: review?(benj)
This function returns the typed array element type that corresponds to the
lanes in a SIMD vector type. It is used to handle SIMD loads and stores from a
typed array.
Attachment #8689123 - Flags: review?(benj)
Ben, we could also rename s/AsmSimdTypeToScalarType/AsmSimdTypeToLaneType/. What do you think?
Comment on attachment 8689122 [details] [diff] [review]
SimdTypeToLaneType.

Review of attachment 8689122 [details] [diff] [review]:
-----------------------------------------------------------------

That's better, thank you for the patch.
Attachment #8689122 - Flags: review?(benj) → review+
Comment on attachment 8689123 [details] [diff] [review]
SimdTypeToArrayElementType.

Review of attachment 8689123 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, that's indeed better.
Attachment #8689123 - Flags: review?(benj) → review+
(In reply to Jakob Stoklund Olesen [:jolesen] from comment #4)
> Ben, we could also rename s/AsmSimdTypeToScalarType/AsmSimdTypeToLaneType/.
> What do you think?

That would work for me. We've used Scalar at several locations to refer to the lane type (in opposition to the vector type), but saying Lane is clearer, in my opinion.
Rename AsmSimdTypeToScalarType to AsmSimdTypeToLaneType, hoping that the new
name will be clearer.
Attachment #8689589 - Flags: review?(benj)
Comment on attachment 8689589 [details] [diff] [review]
AsmSimdTypeToLaneType.

Review of attachment 8689589 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8689589 - Flags: review?(benj) → review+
You need to log in before you can comment on or make changes to this bug.