This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Fix a number of inefficiencies and issues with atomic code gen
ClosedPublic

Authored by nemanjai on Sep 27 2022, 7:27 PM.

Details

Summary

There are a few issues with the code we generate for atomic operations and the way we generate it:

  • Hard coded CR0 for compares
  • Order of operands for compares not conducive to emitting compare-immediate or for CSE of compares
  • Missing MachineMemOperand for st[bhwd]cx intrinsics
  • A missing intrinsic property for the same
  • Unnecessary blocks with store conditional instructions to clear reservation (which ends up hindering performance)
  • Move from CR instructions just to compare the result of a store conditional with zero (even though it is a record-form)

This patch aims to resolve all of those issues.

Diff Detail

Event Timeline

nemanjai created this revision.Sep 27 2022, 7:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2022, 7:27 PM
nemanjai requested review of this revision.Sep 27 2022, 7:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2022, 7:27 PM
lkail added a subscriber: lkail.Sep 29 2022, 8:25 PM
lkail added inline comments.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
15754

Is it missing a test, kinda like

define signext i1 @foo(i64* %addr, i64 %newval) {
entry:
  %0 = bitcast i64* %addr to i8*
  br label %while.cond

while.cond:                                       ; preds = %while.body, %entry
  %1 = tail call i32 @llvm.ppc.stdcx(i8* %0, i64 %newval)
  %ok = icmp eq i32 %1, 0
  br i1 %ok, label %end, label %end.0
end:
  ret i1 0
end.0:
  ret i1 1
}
nemanjai added inline comments.Oct 3 2022, 7:53 AM
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
15754

We have it in test/CodeGen/PowerPC/builtins-ppc-xlcompat-check-ldarx-opt.ll but I agree that it would probably be a good idea to add a simplified test case like this for each of the sizes. I'll add one for each of byte/halfword/word/doubleword.

nemanjai updated this revision to Diff 464723.Oct 3 2022, 10:00 AM

Add test cases specifically for branching on store conditional.

RolandF accepted this revision.Oct 3 2022, 1:33 PM

LGTM

This revision is now accepted and ready to land.Oct 3 2022, 1:33 PM
kbarton added inline comments.Oct 3 2022, 1:48 PM
llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
1380

What about ppc_sthcx and ppc_stbcx?

nemanjai added inline comments.Oct 3 2022, 3:34 PM
llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
1380

Ah good catch. I missed the subword ones. I'll add them.