This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Merge FMV_H_X_RV32/FMV_H_X_RV64 into a single opcode. Same with FMV_X_ANYEXTH_RV32/RV64
ClosedPublic

Authored by craig.topper on Dec 2 2020, 8:56 PM.

Details

Summary

Rather than having a different opcode for RV32 and RV64. Let's just say the integer type is XLenVT and use a single opcode for both modes.

Diff Detail

Event Timeline

craig.topper created this revision.Dec 2 2020, 8:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2020, 8:56 PM
craig.topper requested review of this revision.Dec 2 2020, 8:56 PM

Makes sense to me otherwise but I'll defer my LGTM to someone else.

llvm/lib/Target/RISCV/RISCVISelLowering.h
56–57

nit: the comments for these notes are still third-person plural despite each being a single node: match and are; I think matches and is is better.

Update comments.

jrtc27 added inline comments.Dec 3 2020, 9:34 AM
llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td
344

Hm, looking at the other files this really should have had a blank line above it, which is more important now you're adding a new set of patterns to the conversion operations "section".

348

This could do with a comment; F has // Moves (no conversion) for its bitcasts.

Address @jrtc27 's comments

jrtc27 accepted this revision.Dec 3 2020, 10:56 AM

Ah, I hadn't realised the F comment got lost in 741b04b0b7912611a8a5b7e74462e87b8930a116, I was looking at our fork which is currently still based on 11. This looks good to me now.

This revision is now accepted and ready to land.Dec 3 2020, 10:56 AM

Ah, I hadn't realised the F comment got lost in 741b04b0b7912611a8a5b7e74462e87b8930a116, I was looking at our fork which is currently still based on 11. This looks good to me now.

I'm not sure why I dropped it unless I was basing it on the lack of comment for the RV64 patterns.