This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add patterns for vector widening integer multiply
ClosedPublic

Authored by jacquesguan on Jan 14 2022, 10:30 PM.

Details

Diff Detail

Event Timeline

jacquesguan created this revision.Jan 14 2022, 10:30 PM
jacquesguan requested review of this revision.Jan 14 2022, 10:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 14 2022, 10:30 PM
eopXD added a subscriber: eopXD.Jan 14 2022, 10:33 PM

Hi JianJian,

I see 12.14 Integer Widening Multiply-Add not added yet, may I give it a try?

Best regards,

Yueh-Ting (eop) Chen

Jianjian Guan via Phabricator <reviews@reviews.llvm.org> 於 2022年1月15日 下午2:30 寫道:

jacquesguan created this revision.
jacquesguan added reviewers: craig.topper, asb, luismarques, frasercrmck, HsiangKai, khchen, benshi001.
Herald added subscribers: VincentWu, luke957, achieveartificialintelligence, vkmr, evandro, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya.
jacquesguan requested review of this revision.
Herald added subscribers: llvm-commits, MaskRay.
Herald added a project: LLVM.

Add patterns for vector widening integer multiply instructions

Repository:
rG LLVM Github Monorepo

https://reviews.llvm.org/D117385

Files:
llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
llvm/test/CodeGen/RISCV/rvv/vwmul-sdnode.ll

<D117385.400241.patch>

Hi JianJian,

I see 12.14 Integer Widening Multiply-Add not added yet, may I give it a try?

Best regards,

Yueh-Ting (eop) Chen

Jianjian Guan via Phabricator <reviews@reviews.llvm.org> 於 2022年1月15日 下午2:30 寫道:

jacquesguan created this revision.
jacquesguan added reviewers: craig.topper, asb, luismarques, frasercrmck, HsiangKai, khchen, benshi001.
Herald added subscribers: VincentWu, luke957, achieveartificialintelligence, vkmr, evandro, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya.
jacquesguan requested review of this revision.
Herald added subscribers: llvm-commits, MaskRay.
Herald added a project: LLVM.

Add patterns for vector widening integer multiply instructions

Repository:
rG LLVM Github Monorepo

https://reviews.llvm.org/D117385

Files:
llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
llvm/test/CodeGen/RISCV/rvv/vwmul-sdnode.ll

<D117385.400241.patch>

Of course, you can, thanks.

What about vwmulsu?

I am curious what we should do for something like (nxvXi32 (mul (sext (nxvXi8 X)), (sext (nxvXi8 Y)). Should we leave the sexts alone or should be shrink them to (nxvXi16 (sext (nxvXi8 X)) and use a widening multiply to do the rest of the extend?

Add patterns for VWMULSU

What about vwmulsu?

I am curious what we should do for something like (nxvXi32 (mul (sext (nxvXi8 X)), (sext (nxvXi8 Y)). Should we leave the sexts alone or should be shrink them to (nxvXi16 (sext (nxvXi8 X)) and use a widening multiply to do the rest of the extend?

I think that maybe (nxvXi32 (mul (sext (nxvXi8 X)), (sext (nxvXi8 Y)) -> (nxvXi32 sext (mul (nxvXi16 (sext (nxvXi8 X))), (nxvXi16 (sext (nxvXi8 Y))))) is better? In this way, we could use less width vector to calaculate, it could be faster?

What about vwmulsu?

I am curious what we should do for something like (nxvXi32 (mul (sext (nxvXi8 X)), (sext (nxvXi8 Y)). Should we leave the sexts alone or should be shrink them to (nxvXi16 (sext (nxvXi8 X)) and use a widening multiply to do the rest of the extend?

I think that maybe (nxvXi32 (mul (sext (nxvXi8 X)), (sext (nxvXi8 Y)) -> (nxvXi32 sext (mul (nxvXi16 (sext (nxvXi8 X))), (nxvXi16 (sext (nxvXi8 Y))))) is better? In this way, we could use less width vector to calaculate, it could be faster?

That's a good point. I guess my question better applies to the case where one type is 16 bits, the other is 8 bits and the result is 32 bits.

craig.topper added inline comments.Jan 17 2022, 11:13 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
651

I'm a bit skeptical that we would see an anyext before a multiply, but if we're going to handle anyext, then we should probably handle one operand being a zext and the other being an anyext.

What about vwmulsu?

I am curious what we should do for something like (nxvXi32 (mul (sext (nxvXi8 X)), (sext (nxvXi8 Y)). Should we leave the sexts alone or should be shrink them to (nxvXi16 (sext (nxvXi8 X)) and use a widening multiply to do the rest of the extend?

I think that maybe (nxvXi32 (mul (sext (nxvXi8 X)), (sext (nxvXi8 Y)) -> (nxvXi32 sext (mul (nxvXi16 (sext (nxvXi8 X))), (nxvXi16 (sext (nxvXi8 Y))))) is better? In this way, we could use less width vector to calaculate, it could be faster?

That's a good point. I guess my question better applies to the case where one type is 16 bits, the other is 8 bits and the result is 32 bits.

For this case, I think maybe extend the 8-bits vector to 16-bits vector firstly, and then use the widen instruction to get the 32-bits result.

Address comment.

llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
651

Done.

craig.topper added inline comments.Jan 17 2022, 8:13 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
657

You don't need to handle both orders or zext/anyext. Tablegen knows mul is commutable and will autogenerate the swapped version. Sorry I suspected you might not know that and should have mentioned it.

Address comment

llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
657

Done, thanks a lot.

Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2022, 7:33 PM
This revision is now accepted and ready to land.Mar 23 2022, 11:34 PM
This revision was landed with ongoing or failed builds.Mar 24 2022, 12:26 AM
This revision was automatically updated to reflect the committed changes.