Page MenuHomePhabricator

[VE] Add integer arithmetic vector instructions
ClosedPublic

Authored by kaz7 on Oct 18 2020, 3:15 AM.

Details

Summary

Add VADD/VADS/VADX/VSUB/VSBS/VSBX/VMPY/VMPS/VMPX/VMPD/VDIV/VDVS/VDVX
instructions. Also add regression tests.

Diff Detail

Event Timeline

kaz7 created this revision.Oct 18 2020, 3:15 AM
kaz7 requested review of this revision.Oct 18 2020, 3:15 AM
simoll added inline comments.Oct 19 2020, 2:14 AM
llvm/lib/Target/VE/VEInstrVec.td
507

Shouldn't the RC register class for pv.*.up instructions be F32?

kaz7 added a comment.Oct 19 2020, 2:48 AM

Make sense. Let me check them again. Thanks.

kaz7 planned changes to this revision.Oct 19 2020, 4:18 AM

Add reason and idea.

llvm/lib/Target/VE/VEInstrVec.td
507

I remember why I use I64 here. This instruction is vector integer add instruction. I didn't use F32 here since our F32 has represented only f32 values. It doesn't represent i32 values.

However, it may be possible to assign not only f32 but also i32 to F32 register class and use F32 here. Similarly, it is also possible to assign both f32 and i32 to I32 register class too since VE has vector floating-point add instruction like "PVFADDLO".

Let me consider this.

simoll added inline comments.Oct 19 2020, 5:18 AM
llvm/lib/Target/VE/VEInstrVec.td
507

I like your idea of mapping f32/i32 to both I32 and F32. AFAIK, the mapping between types (eg i32) and registers (eg F32) only matters for isel pattern constraints (custom lowering code is not subject to those constraints).

kaz7 requested review of this revision.Oct 20 2020, 5:47 AM

Change my mind. Stop previous plan to change.

llvm/lib/Target/VE/VEInstrVec.td
507

I like that idea too. But, I understand doing that needs a lot of work. Can we merge this one as is and try to optimize it later?

For example, it is possible to define f32 and i32 to both F32 and I32. However, it doesn't help ISel patterns at the moment. Let consider about implementing AND for higher i32 and lower i32. What kind of patterns we can to write? Pat<(i32 (and ... doesn't work. Maybe we can write Pat<(I32 (and... and Pat<(F32 (and.... I've not tried it yet. Then, we also need a lot of patterns to make it worth.

So, let me merge all vector instructions first.

kaz7 abandoned this revision.Oct 21 2020, 4:58 AM

It looks like all of these modifications will be overwritten by internal modifications, so I'm abandoning all of vector modifications once.

kaz7 reclaimed this revision.Oct 23 2020, 6:11 AM
simoll accepted this revision.Oct 26 2020, 2:26 AM
This revision is now accepted and ready to land.Oct 26 2020, 2:26 AM
This revision was landed with ongoing or failed builds.Oct 26 2020, 2:30 AM
This revision was automatically updated to reflect the committed changes.