This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Fix for indexed dot product instruction descriptions
ClosedPublic

Authored by SjoerdMeijer on Sep 18 2017, 3:54 AM.

Details

Summary

The indexed dot product instructions only accept the lower 16 D-registers as
the indexed register, but we were e.g. incorrectly accepting:

vudot.u8 d16,d16,d18[0]

Diff Detail

Repository
rL LLVM

Event Timeline

SjoerdMeijer created this revision.Sep 18 2017, 3:54 AM
rengolin edited edge metadata.Sep 18 2017, 4:13 AM

The last ARMv8.2 manual I could find is from 31 March, but it says UDOT/SDOT will be documented later. Do you have an update on that?

It's really hard to review patches without official documentation out.

Hi, the full architecture specification is publicly available here:

https://developer.arm.com/products/architecture/a-profile/exploration-tools

which I also mentioned in the commit message of r310480, which introduced initial AArch64 assembler support.

Hope this helps.

Interesting, the Aarch32 PDF doesn't have SDOT/UDOT, only the AArch64 one...

Ignore me, that was VSDOT... :)

Where is the restriction that only the lower 16 registers are allowed? Can you test quad regs, too? Just to make it clear the lane issue.

t.p.northover edited edge metadata.Sep 18 2017, 7:01 AM

Where is the restriction that only the lower 16 registers are allowed? Can you test quad regs, too? Just to make it clear the lane issue.

It's in the pseudo-code:

integer m = UInt(Vm<3:0>);
integer index = UInt(M);

Normally that 'M' bit would be the high bit of Vm (as for Vd and Vn just above). Here it's used to encode the lane.

I was just replying, but yes, there are only 4 bits available to encode Vm, the other M bit is the index.

rengolin accepted this revision.Sep 18 2017, 7:07 AM

It's in the pseudo-code:

D'oh! I was looking in the vector one.

LGTM. Thanks!

This revision is now accepted and ready to land.Sep 18 2017, 7:07 AM

Thanks for checking!

This revision was automatically updated to reflect the committed changes.