This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Add fix to partword atomic operations
ClosedPublic

Authored by stefanp on May 19 2021, 6:25 PM.

Details

Reviewers
nemanjai
lei
Group Reviewers
Restricted Project
Commits
rG45ad207e4585: [PowerPC] Add fix to partword atomic operations
Summary

Partword atomic binaries are not zero extended as they should be.
This patch fixes them to ensure that they are zero extended.

Diff Detail

Event Timeline

stefanp created this revision.May 19 2021, 6:25 PM
stefanp requested review of this revision.May 19 2021, 6:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2021, 6:25 PM
nemanjai accepted this revision.May 19 2021, 7:01 PM

LGTM. Of course, please ensure that this fixes the issue when the dependent patch is applied prior to committing this.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp
11359–11366
// Since the shift amount is not a constant, we need to clear
// the upper bits with a separate RLWINM.
llvm/test/CodeGen/PowerPC/ppc-partword-atomic.ll
104

Presumably this goes away when https://reviews.llvm.org/D101342 lands? Can you please apply that on top of this patch to make sure? Similarly with line 49.

This revision is now accepted and ready to land.May 19 2021, 7:01 PM
stefanp updated this revision to Diff 346789.May 20 2021, 10:27 AM

Added code comment.

stefanp added inline comments.May 20 2021, 10:31 AM
llvm/test/CodeGen/PowerPC/ppc-partword-atomic.ll
104

Presumably this goes away when https://reviews.llvm.org/D101342 lands? Can you please apply that on top of this patch to make sure? Similarly with line 49.

I have checked this and if I add the patch D101342 then those two lines disappear.
Here is the diff in the test case with the patch added:

--- a/llvm/test/CodeGen/PowerPC/ppc-partword-atomic.ll
+++ b/llvm/test/CodeGen/PowerPC/ppc-partword-atomic.ll
@@ -46,10 +46,9 @@ define dso_local zeroext i32 @testI8(i8 zeroext %val) local_unnamed_addr #0 {
 ; PWR9-NEXT:    stbcx. 3, 0, 5
 ; PWR9-NEXT:    bne 0, .LBB0_1
 ; PWR9-NEXT:  # %bb.2: # %entry
-; PWR9-NEXT:    clrlwi 3, 4, 24
-; PWR9-NEXT:    addis 4, 2, global_int@toc@ha
+; PWR9-NEXT:    addis 3, 2, global_int@toc@ha
 ; PWR9-NEXT:    lwsync
-; PWR9-NEXT:    stw 3, global_int@toc@l(4)
+; PWR9-NEXT:    stw 4, global_int@toc@l(3)
 ; PWR9-NEXT:    li 3, 55
 ; PWR9-NEXT:    blr
 entry:
@@ -100,10 +99,9 @@ define dso_local zeroext i32 @testI16(i16 zeroext %val) local_unnamed_addr #0 {
 ; PWR9-NEXT:    sthcx. 3, 0, 5
 ; PWR9-NEXT:    bne 0, .LBB1_1
 ; PWR9-NEXT:  # %bb.2: # %entry
-; PWR9-NEXT:    clrlwi 3, 4, 16
-; PWR9-NEXT:    addis 4, 2, global_int@toc@ha
+; PWR9-NEXT:    addis 3, 2, global_int@toc@ha
 ; PWR9-NEXT:    lwsync
-; PWR9-NEXT:    stw 3, global_int@toc@l(4)
+; PWR9-NEXT:    stw 4, global_int@toc@l(3)
 ; PWR9-NEXT:    li 3, 55
 ; PWR9-NEXT:    blr
 entry:
This revision was landed with ongoing or failed builds.May 20 2021, 10:36 AM
This revision was automatically updated to reflect the committed changes.