This is an archive of the discontinued LLVM Phabricator instance.

[LoongArch] Construct codegen infra and generate first add instruction.
ClosedPublic

Authored by wangleiat on Mar 23 2022, 6:36 PM.

Details

Summary

This patch constructs codegen infra and successfully generate the first
'add' instruction. Add integer calling convention for fixed arguments which
are passed with general-purpose registers.

New test added here:

CodeGen/LoongArch/ir-instruction/add.ll

The test file is placed in a subdirectory because we will use
subdirctories to distinguish different categories of tests (e.g.
intrinsic, inline-asm ...)

Diff Detail

Event Timeline

wangleiat created this revision.Mar 23 2022, 6:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2022, 6:36 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
wangleiat requested review of this revision.Mar 23 2022, 6:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2022, 6:36 PM
wangleiat updated this revision to Diff 417809.Mar 23 2022, 6:58 PM

Delete useless comments.
Change 'check-prefixes' to check-prefix, because only one check-prefix is needed in the current test.

Hi @MaskRay @xen0n @rengolin @myhsu,

The author (@wangleiat) of this patch is one of the LoongArch port developers from Loongson Inc. This patch has been reviewed internally before being uploaded. We hope you could raise more comments. Without your approval I will not commit it to github especially we are in an early stage.

Thanks,
Weining

MaskRay added inline comments.Mar 27 2022, 11:19 PM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
51

Consider using X macro here to avoid future boilerplate.

92

llvm_unreachable takes a string literal. Use "" instead of nullptr if you don't have a specific message.

102

Coding standard uses pre-increments.

142

Is 16 necessary?

Suggest SmallVector<CCValAssign> ArgLocs;.

158

return ... <= 2;

llvm/lib/Target/LoongArch/LoongArchISelLowering.h
63

https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments
"Don’t duplicate function or class name at the beginning of the comment."

65

Prefer using to typedef.

Does llvm/CodeGen/CallingConvLower.h CCAssignFn work for you or you just want to remove the ISD::ArgFlagsTy argument?

llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp
39
40

llvm_unreachable does need a subsequent return.

llvm/test/CodeGen/LoongArch/ir-instruction/add.ll
2

--verify-machineinstrs is not necessary.

LLVM_ENABLE_EXPENSIVE_CHECKS=on builds default to --verify-machineinstrs unless the target sets isMachineVerifierClean to false.

wangleiat added inline comments.Mar 28 2022, 3:24 AM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
51

Good idea. Thanks, i will change that.

92

Thanks.

102

OK, thanks. It seems that i have to learn coding standards first. :)

142

OK. I will change that.

158

Thanks.

llvm/lib/Target/LoongArch/LoongArchISelLowering.h
63

OK, i will remove it.

65

To define a target-specific function, the main reason is that more information needs to be passed to. For example, the abi information to be added in the future. (loongarch calling convention is very similar to risc-v.)

llvm/lib/Target/LoongArch/LoongArchInstrInfo.cpp
39

OK, thanks.

40

Thanks, i will remove it.

llvm/test/CodeGen/LoongArch/ir-instruction/add.ll
2

Thanks. I will remove them.

wangleiat updated this revision to Diff 418534.Mar 28 2022, 3:50 AM

Fix coding style issues.

SixWeining added inline comments.Mar 29 2022, 5:00 AM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
57

redundant semicolon

rcorcs added a subscriber: rcorcs.Mar 29 2022, 5:03 AM
wangleiat updated this revision to Diff 419082.Mar 30 2022, 2:43 AM

Remove a useless ;

wangleiat updated this revision to Diff 419084.Mar 30 2022, 2:49 AM

modify the wrong macro name

This revision is now accepted and ready to land.Mar 30 2022, 8:11 PM
MaskRay accepted this revision.Mar 30 2022, 8:45 PM
This revision was landed with ongoing or failed builds.Mar 30 2022, 9:04 PM
This revision was automatically updated to reflect the committed changes.

This patch has been rebased and pushed. Thanks.