This is an archive of the discontinued LLVM Phabricator instance.

[LoongArch] Use tablegen size for getInstSizeInBytes
ClosedPublic

Authored by XiaodongLoong on Nov 21 2022, 7:28 PM.

Details

Summary

Correct the pseudo atomic instruction size for branch
relaxation and branch folding passes.

Inspired by D118175, D118009 and D117970.

Depends on D138481

Diff Detail

Event Timeline

XiaodongLoong created this revision.Nov 21 2022, 7:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 21 2022, 7:28 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
XiaodongLoong requested review of this revision.Nov 21 2022, 7:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 21 2022, 7:28 PM
SixWeining accepted this revision.Nov 22 2022, 5:58 AM

LGTM. But tt's better be checked by @gonglingqin who wrote most of the AMO pseudos.

llvm/unittests/Target/LoongArch/InstSizes.cpp
118–130

This is less readable. Could you write each instruction in one line. If clang-format fail, clang-format off/on may help.

This revision is now accepted and ready to land.Nov 22 2022, 5:58 AM

And maybe this need to be sequenced after D138481?

More readable.

XiaodongLoong edited the summary of this revision. (Show Details)Nov 22 2022, 11:31 PM
XiaodongLoong marked an inline comment as done.

And maybe this need to be sequenced after D138481?

Yes, you are right.

xen0n accepted this revision.Nov 30 2022, 5:24 PM

Update for AMMax and AMMin

Rebase code.

This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Dec 3 2022, 4:04 AM
thakis added inline comments.
llvm/unittests/Target/LoongArch/CMakeLists.txt
11

Do you need all these dependencies? From a quick look, InstSizes.cpp seems to not include files from most of these.

thakis added a comment.Dec 3 2022, 4:39 AM

The test added in this change fails:

[ RUN      ] InstSizes.AtomicPseudo

# Machine code for function sizes: IsSSA, NoPHIs, NoVRegs
Jump Tables:
%jump-table.0: %bb.0


bb.0:
  dead early-clobber renamable $r10, dead early-clobber renamable $r11 = PseudoMaskedAtomicLoadAdd32 renamable $r7, renamable $r6, renamable $r8, 4
  dead early-clobber renamable $r10, dead early-clobber renamable $r11 = PseudoAtomicLoadAdd32 renamable $r7, renamable $r6
  dead early-clobber renamable $r5, dead early-clobber renamable $r9, dead early-clobber renamable $r10 = PseudoMaskedAtomicLoadUMax32 renamable $r7, renamable $r6, renamable $r8, 4
  early-clobber renamable $r9, dead early-clobber renamable $r10, dead early-clobber renamable $r11 = PseudoMaskedAtomicLoadMax32 killed renamable $r6, killed renamable $r5, killed renamable $r7, killed renamable $r8, 4
  dead early-clobber renamable $r5, dead early-clobber renamable $r9 = PseudoCmpXchg32 renamable $r7, renamable $r4, renamable $r6
  dead early-clobber renamable $r5, dead early-clobber renamable $r9 = PseudoMaskedCmpXchg32 killed renamable $r7, killed renamable $r4, killed renamable $r6, killed renamable $r8, 4

# End machine code for function sizes.

*** Bad machine code: Too few operands ***
- function:    sizes
- basic block: %bb.0  (0x14181e4c8)
- instruction: dead early-clobber renamable $r10, dead early-clobber renamable $r11 = PseudoAtomicLoadAdd32 renamable $r7, renamable $r6
5 operands expected, but 4 given.
LLVM ERROR: Found 1 machine code errors.

Please take a look and revert for now if it takes a while to fix.

@thakis OK, Thank you a lot.

XiaodongLoong reopened this revision.Dec 4 2022, 6:19 PM
This revision is now accepted and ready to land.Dec 4 2022, 6:19 PM

Fix the test error.

XiaodongLoong marked an inline comment as done.Dec 4 2022, 6:23 PM
XiaodongLoong added inline comments.
llvm/unittests/Target/LoongArch/CMakeLists.txt
11

Thanks, I delete the redundant dependencies.

XiaodongLoong marked an inline comment as done.

Rebase code.

This revision was landed with ongoing or failed builds.Dec 6 2022, 11:51 PM
This revision was automatically updated to reflect the committed changes.