This is an archive of the discontinued LLVM Phabricator instance.

[LoongArch] Add remaining intrinsics for CRC check instructions
ClosedPublic

Authored by gonglingqin on Nov 21 2022, 4:37 AM.

Details

Summary

After D137316 implements the intrinsics of the first crc check instruction
and related diagnosis, this patch implements the intrinsics of all remaining
crc check instructions.

Diff Detail

Event Timeline

gonglingqin created this revision.Nov 21 2022, 4:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 21 2022, 4:37 AM
gonglingqin requested review of this revision.Nov 21 2022, 4:37 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 21 2022, 4:37 AM
SixWeining added inline comments.Nov 22 2022, 5:54 AM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
953–961

Can we define a marco but not repeating similar codes?
For example:

#define CRC_CASE(NAME, ISD) \
    case Intrinsic::loongarch_#NAME: { \
      Results.push_back(DAG.getNode( \
          ISD::TRUNCATE, DL, VT, \
          DAG.getNode(LoongArchISD::#ISD, DL, MVT::i64, \
                      DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, Op2), \
                      DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, Op3)))); \
      Results.push_back(N->getOperand(0)); \
      break; \
    }

Then

CRC_CASE(crc_w_b_w, CRC_W_B_W)

Address @SixWeining's comments.

SixWeining accepted this revision.Nov 22 2022, 10:47 PM

LGTM.

llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
953–980

You could use uppercase to keep consistency.

This revision is now accepted and ready to land.Nov 22 2022, 10:47 PM
gonglingqin added inline comments.Nov 22 2022, 10:56 PM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
953–980

Thanks, I will modify it.

Address @SixWeining's comments.

otherwise LGTM

llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
953–980

What's a "two op", are you meaning "binary op" instead?

970

Similarly, is this actually "unary op"?

gonglingqin added inline comments.Nov 22 2022, 11:39 PM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
953–980

Yes, thank you for your correction!

970

Thanks a lot! I will modify it.

Address @xen0n's comments.

SixWeining added inline comments.Nov 22 2022, 11:54 PM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
953–980

Seems the difference is not binary op or unar op. They are both binary ops. The difference is that the former uses 2 ISD::ANY_EXTEND while the latter only use one.

SixWeining added inline comments.Nov 27 2022, 5:14 PM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
953–980

@xen0n What do you think?

This revision was landed with ongoing or failed builds.Nov 30 2022, 5:47 PM
This revision was automatically updated to reflect the committed changes.