Open Bug 1216211 Opened 9 years ago Updated 2 years ago

Add support for different constant pool alignments

Categories

(Core :: JavaScript Engine: JIT, defect, P5)

ARM
Unspecified
defect

Tracking

()

People

(Reporter: jolesen, Unassigned)

References

Details

The AssemblerBufferWithConstantPools class is allocating constant pool entries in multiples on |PoolAllocUnit| which is usually an |uint32_t|. This is also the guaranteed alignment (at least as long as it happens to be identical to the instruction size).

Some micro-architecture implementations of ARM and ARM64 could benefit from constant pool entries with larger alignment: 16-byte aligned SIMD vector constants, and 8-byte aligned ARM64 pointers, for example.

Instead of having a single fixed PoolAllocUnit, we should allow assemblers to create constant pool entries of any byte size with any power-of-two alignment. The machinery to support this is quite simple and unlikely to have a measurable compile time impact, and there could be runtime improvements, at least on some chips.
Also address this review comment from bug 1207827:

> Comment on attachment 8674987 [details] [diff] [review]
> Switch jit::Pool to a LifoAllocPolicy.
> 
> Review of attachment 8674987 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jit/shared/IonAssemblerBufferWithConstantPools.h
> @@ +181,5 @@
> >      unsigned insertEntry(unsigned num, uint8_t* data, BufferOffset off, LifoAlloc& lifoAlloc) {
> >          if (oom_)
> >              return OOM_FAIL;
> > +        unsigned ret = numEntries();
> > +        if (!poolData_.append((PoolAllocUnit*)data, num) || !loadOffsets.append(off)) {
> 
> We should probably open a follow-up bug to get rid of this cast.

The natural type for pool entry data is (const void *data, size_t bytes).
See Also: → 1207827
Priority: -- → P5
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.