Page MenuHomePhabricator

[RISCV] Add intrinsics for vector AMO instructions
ClosedPublic

Authored by arcbbb on Jan 13 2021, 4:16 AM.

Details

Summary

Add vamoswap, vamoadd, vamoxor, vamoand, vamoor,
vamomin, vamomax, vamominu, vamomaxu intrinsics.

Diff Detail

Event Timeline

arcbbb created this revision.Jan 13 2021, 4:16 AM
arcbbb requested review of this revision.Jan 13 2021, 4:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2021, 4:16 AM
khchen added a subscriber: khchen.Jan 13 2021, 7:46 AM
khchen added inline comments.
llvm/include/llvm/IR/IntrinsicsRISCV.td
589

based on https://reviews.llvm.org/D93359#inline-872142
Could we do the similar thing like vlxe/vsxe did here?

arcbbb marked an inline comment as done.Jan 13 2021, 6:16 PM
arcbbb added inline comments.
llvm/include/llvm/IR/IntrinsicsRISCV.td
589

Good idea, thanks!

arcbbb updated this revision to Diff 316546.Jan 13 2021, 6:27 PM
arcbbb marked an inline comment as done.
arcbbb edited the summary of this revision. (Show Details)

address @khchen's comment

arcbbb updated this revision to Diff 316586.Jan 14 2021, 1:06 AM

add double type for vamoswap-rv32.ll

asb added a comment.Jan 14 2021, 6:35 AM

Nitpick: this would be better titled "[RISCV] Add intrinsics for vector AMO instructions" - I was a little confused seeing the title come past :)

arcbbb retitled this revision from [RISCV] Add intrinsics for AMO instructions to [RISCV] Add intrinsics for vector AMO instructions.Jan 14 2021, 9:07 AM
In D94589#2498139, @asb wrote:

Nitpick: this would be better titled "[RISCV] Add intrinsics for vector AMO instructions" - I was a little confused seeing the title come past :)

Ah, sorry for the confusing. I have changed the title.

khchen added inline comments.Jan 15 2021, 7:10 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
107

vlxe and vlse have the same logic and I think they can reuse above utility functions, right?
Do you have a plan to update them (vlxe and vlse) and make the same logic with consistent implementation? maybe it could be in another patch.

3216

Should it have HasStdExtV predicate?

craig.topper added inline comments.Jan 15 2021, 4:04 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
3216

I think FeatureExtZvamo implies HasStdExtV in RISCV.td

craig.topper added inline comments.Jan 15 2021, 4:12 PM
llvm/include/llvm/IR/IntrinsicsRISCV.td
476

Is the value the same type as the result? Can we use LLVMMatchType<0> for it?

arcbbb updated this revision to Diff 317167.Jan 16 2021, 4:46 AM
  1. fix value type to match result type
  2. share octuple utility function with vlxei & vsxei patterns
arcbbb marked 2 inline comments as done.Jan 16 2021, 4:49 AM
arcbbb added inline comments.
llvm/include/llvm/IR/IntrinsicsRISCV.td
476

yes, thanks!

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
107

sure!

khchen added inline comments.Jan 16 2021, 6:37 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
905

Why is this !sra not !srl?

2542

same here.

arcbbb updated this revision to Diff 317254.Jan 17 2021, 6:43 PM
arcbbb marked 2 inline comments as done.

changed sra to srl

khchen accepted this revision.Jan 18 2021, 7:03 PM

LGTM

This revision is now accepted and ready to land.Jan 18 2021, 7:03 PM
This revision was automatically updated to reflect the committed changes.