This is an archive of the discontinued LLVM Phabricator instance.

[LoongArch] Add LSX intrinsic support
ClosedPublic

Authored by wangleiat on Jul 20 2023, 5:52 AM.

Details

Summary

For handling intrinsics, our approach is not simply to match them
one-to-one with instructions. Instead, we lower some intrinsics
to common nodes and then perform matching. The advantage of this
approach is that it allows us to fully utilize the passes available
at the common layer for optimizing purposes.

We perform error checks on the immediate operand of all intrinsics,
rather than waiting until the end to throw exceptions.

Diff Detail

Event Timeline

wangleiat created this revision.Jul 20 2023, 5:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2023, 5:52 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
wangleiat requested review of this revision.Jul 20 2023, 5:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2023, 5:52 AM
xen0n added a comment.Jul 20 2023, 6:16 AM

Just took a quick look mainly checking for stylistic concerns and things look good at first glance! I'd like to do a closer review but I may not be able to adequately finish it before the LLVM 17 branching next week. I'll try though (to get more people looking here perhaps).

llvm/lib/Target/LoongArch/LoongArchISelLowering.h
64–75

Put those below the `Intrinsic operations start" marker perhaps?

llvm/lib/Target/LoongArch/LoongArchLSXInstrInfo.td
20

Make the pattern name loongarch_vall_nonzero for consistency with the C++ name? Same for other similar cases.

1526

Can this !tolower call be lifted into a helper class like deriveInsnMnemonic at least? (I don't know if the whole !cast<Intrinsic> expression could be abstracted this way, or even the whole Pat def.)

wangleiat marked 3 inline comments as done.Jul 20 2023, 7:29 PM

Thanks.

wangleiat updated this revision to Diff 542746.Jul 20 2023, 7:31 PM

Address @xen0n's comments.

SixWeining added inline comments.Jul 23 2023, 8:31 PM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
829

simm13 is used in instruction definition (LoongArchLSXInstrInfo.td) while uimm13 is used here. For consistency, it's better to use simm13 (binutils also uses simm13).

wangleiat added inline comments.Jul 23 2023, 8:54 PM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
829

Thank you, I will make the modification.

wangleiat updated this revision to Diff 543361.Jul 23 2023, 8:57 PM

Address @SixWeining's comments.

This revision is now accepted and ready to land.Aug 3 2023, 7:21 AM
This revision was automatically updated to reflect the committed changes.