Page MenuHomePhabricator

[LoongArch] Add codegen support for atomicrmw min/max operation on LA64
ClosedPublic

Authored by gonglingqin on Nov 16 2022, 7:33 PM.

Details

Summary

This patch is required by OpenMP. After applying this patch, OpenMP regression
test passed. To reduce review difficulty caused by too large patches,
atomicrmw min/max operations on LA32 will be added later.

Diff Detail

Event Timeline

gonglingqin created this revision.Nov 16 2022, 7:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 16 2022, 7:33 PM
gonglingqin requested review of this revision.Nov 16 2022, 7:33 PM
Herald added a project: Restricted Project. · View Herald Transcript

Please rebase code to latest commit. the failed test should be OK.

llvm/lib/Target/LoongArch/LoongArchExpandAtomicPseudoInsts.cpp
439–440

Align?

447–448

Align?

gonglingqin added inline comments.Nov 18 2022, 10:35 PM
llvm/lib/Target/LoongArch/LoongArchExpandAtomicPseudoInsts.cpp
439–440

Thanks, I'll normalize the code format.

447–448

Thanks, I'll normalize the code format.

Rebase and normalize the code format.

SixWeining accepted this revision.Nov 26 2022, 5:30 AM

LGTM and let's wait for others.

This revision is now accepted and ready to land.Nov 26 2022, 5:30 AM
xen0n added inline comments.Nov 26 2022, 7:06 AM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
2413–2414

This may have room for improvement:

Firstly, this value is statically known at compile-time, so it should probably be an immediate. I looked at the surrounding code and found out some similar expressions like this one, so you may choose to refactor later.

Secondly, we have ext.w.b and ext.w.h that is usable even in LA32 mode, so it may be very worthwhile to special-case these two sizes so that the sign extension can be done in one insn/cycle.

gonglingqin added inline comments.Nov 27 2022, 5:33 PM
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
2413–2414

Thanks for your advice! I will improve the relevant implementation in the later patch.