This is an archive of the discontinued LLVM Phabricator instance.

[Clang][RISCV] Guard vmulh, vsmul correctly
ClosedPublic

Authored by eopXD on Jan 21 2022, 11:29 AM.

Details

Summary

According to v-spec 1.0, vmulh, vmulhu, vmulhsu and vsmul are
NOT supported for EEW=64 in Zve64*.

This patch tries to guard it correctly.

Authored by: Craig Topper <craig.topper@sifive.com> @craig.topper
Co-Authored by: Eop Chen <eop.chen@sifive.com> @eopXD

Diff Detail

Event Timeline

eopXD created this revision.Jan 21 2022, 11:29 AM
eopXD requested review of this revision.Jan 21 2022, 11:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 21 2022, 11:29 AM
eopXD retitled this revision from tmp to [RISCV][Clang] Guard vmulh, vsmul correctly.Jan 21 2022, 11:35 AM
eopXD edited the summary of this revision. (Show Details)
eopXD added a subscriber: craig.topper.
eopXD edited the summary of this revision. (Show Details)Jan 21 2022, 11:37 AM
eopXD retitled this revision from [RISCV][Clang] Guard vmulh, vsmul correctly to [Clang][RISCV] Guard vmulh, vsmul correctly.

I changed the test commands in vmul.c and vsmul.c to use zve64d to make sure I don't miss anything I should move to the created files - vmul-eew64.c and vsmul-eew64.c

craig.topper added inline comments.Jan 21 2022, 12:22 PM
clang/utils/TableGen/RISCVVEmitter.cpp
812

Can you add a comment here. I should have in my original patch.

eopXD updated this revision to Diff 402069.Jan 21 2022, 12:23 PM
eopXD marked an inline comment as done.

Add comment.

craig.topper added inline comments.Jan 21 2022, 11:06 PM
clang/utils/TableGen/RISCVVEmitter.cpp
1323

It's not as useful here. Should put it on line 812 where the condition is checked and explain that a FullMultiply is MULH and SMUL.

eopXD updated this revision to Diff 402181.Jan 21 2022, 11:21 PM
eopXD marked an inline comment as done.

Update comment.

khchen added a subscriber: khchen.Jan 22 2022, 7:41 AM

why the new test filename extension is .c.c?

eopXD updated this revision to Diff 402221.Jan 22 2022, 8:24 AM

Fix filename.

eopXD added a comment.Jan 22 2022, 8:24 AM

why the new test filename extension is .c.c?

Thanks for the reminder.

This revision is now accepted and ready to land.Jan 23 2022, 6:43 PM
eopXD updated this revision to Diff 402395.Jan 23 2022, 9:55 PM

Rebase to latest main.

eopXD updated this revision to Diff 402561.Jan 24 2022, 9:14 AM

Update test case due to rebase.

eopXD updated this revision to Diff 402565.Jan 24 2022, 9:15 AM

Rebase to latest main.

craig.topper accepted this revision.Jan 24 2022, 10:02 AM

Still LGTM

eopXD updated this revision to Diff 402751.Jan 24 2022, 7:41 PM

Rebase with no change, the buildbot halted.

eopXD updated this revision to Diff 402752.Jan 24 2022, 7:48 PM

Update code dues to variable rename from new commits.

This revision was landed with ongoing or failed builds.Jan 25 2022, 10:19 AM
This revision was automatically updated to reflect the committed changes.