[PowerPC] Implement Load and Reserve and Store Conditional Builtins

[Conanap]

Summary

This patch implements the load and reserve and store conditional
builtins for the PowerPC target, in order to have feature parity with
xlC on AIX.

[lkail]

The wording might be inaccurate. It's better to rephrase to 'Load and Reserve and Store Conditional'.

llvm/lib/Target/PowerPC/PPCInstr64Bit.td

1724

IIRC, l(w|d)arx, st(w|d)cx are supported very early and don't need altivec support.

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-atomicLoadStore-64-only.ll

7 ↗

(On Diff #355701)

Is -mcpu=pwr9 necessary?

[amyk]

llvm/include/llvm/IR/IntrinsicsPowerPC.td

1536–1537

Is there an unintended change at the end?

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

2841 ↗

(On Diff #355701)

Why are these patterns in PPCInstrPrefix.td? Shouldn't they be in PPCInstrInfo.td?
Also, you can probably use ForceXForm instead of xoaddr here.

[nemanjai]

llvm/lib/Target/PowerPC/PPCInstr64Bit.td

1724

Agreed. This does not require ISA 2.07 and definitely doesn't require Altivec.

[Conanap]

Moved the pattern definitions and removed unnecessary guard.

[Conanap]

Properly updated the diff with arc.

[Conanap]

Changed test case file names

[Conanap]

Updated C test case name

[Conanap]

Addressed comments

[NeHuang]

Please add the 64 bit only sema check & error test case for the two 64 bit only builtins ldarx and stdcx

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64-only.ll

10 ↗

(On Diff #356533)

remove local_unnamed_addr #0

22 ↗

(On Diff #356533)

remove local_unnamed_addr #0

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll

13

remove local_unnamed_addr #0

31

remove local_unnamed_addr #0

[Conanap]

Added error test cases and sema checking

[Conanap]

Updated 64 bit error test cases

[stefanp]

Overall I think this is fine. I just have a few nits.

clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c

12

Please check that this "entry:" is printed out when asserts are on and when asserts are off you may want to remove it at this point.

I would prefer you check the name of the function instead of "entry". You can use CHECK-LABEL to do that.

clang/test/CodeGen/builtins-ppc-xlcompat-error.c

19

This entire test is for 32 bit Power PC. There is only one run line and that is what is specified.
Why are you checking the macros?

llvm/include/llvm/IR/IntrinsicsPowerPC.td

1529

nit:
Does this go past the 80 char limit?

[Conanap]

Updated IR test cases

[Conanap]

Changed to ForceXForm

[NeHuang]

clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c

2

-S seems redundant.
Any reason we need -O2 for these tests?

clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c

2

same as above.

12

Agreed. We should first do CHECK-LABEL with the function name.

[Conanap]

Updated test cases

[Conanap]

addressed comments

[NeHuang]

Overall LGTM.

[Conanap]

Removed entry from check

[amyk]

Also LGTM overall.

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll

3

nit: Add -ppc-asm-full-reg-names.

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll

4

nit: Add -ppc-asm-full-reg-names.

[nemanjai]

LGTM aside from a small nit.

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll

3

+1

[Conanap]

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll

3

The AIX assembler cannot understand register prefixes, so I'll leave the test cases as is.

[stefanp]

Thank you!
LGTM.

[lkail]

lgtm,thanks.

[thakis]

Looks like this breaks tests: http://45.33.8.238/linux/50465/step_7.txt

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

[Conanap]

In D105236#2858909, @thakis wrote:

Looks like this breaks tests: http://45.33.8.238/linux/50465/step_7.txt

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

Hi! I accidentally included a test case meant for a later patch; it's been removed in this patch here: https://reviews.llvm.org/D105454

[nemanjai]

llvm/include/llvm/IR/IntrinsicsPowerPC.td

1533

I don't know why I missed this in the review, but this is very wrong! This is a load intrinsic that is marked as one that doesn't touch memory.

clang/include/clang/Basic/BuiltinsPPC.def

 BUILTIN(__builtin_ppc_fetch_and_add, "UiUiD*Ui", "")
 BUILTIN(__builtin_ppc_fetch_and_addlp, "ULiULiD*ULi", "")
 BUILTIN(__builtin_ppc_fetch_and_and, "UiUiD*Ui", "")
 BUILTIN(__builtin_ppc_fetch_and_andlp, "ULiULiD*ULi", "")
 BUILTIN(__builtin_ppc_fetch_and_or, "UiUiD*Ui", "")
 BUILTIN(__builtin_ppc_fetch_and_orlp, "ULiULiD*ULi", "")
 BUILTIN(__builtin_ppc_fetch_and_swap, "UiUiD*Ui", "")
 BUILTIN(__builtin_ppc_fetch_and_swaplp, "ULiULiD*ULi", "")
+BUILTIN(__builtin_ppc_ldarx, "LiLiD*", "")
+BUILTIN(__builtin_ppc_lwarx, "iiD*", "")
+BUILTIN(__builtin_ppc_stdcx, "iLiD*Li", "")
+BUILTIN(__builtin_ppc_stwcx, "iiD*i", "")
 
 BUILTIN(__builtin_ppc_get_timebase, "ULLi", "n")
 
 // This is just a placeholder, the types and attributes are wrong.
 BUILTIN(__builtin_altivec_vaddcuw, "V4UiV4UiV4Ui", "")
 
 BUILTIN(__builtin_altivec_vaddsbs, "V16ScV16ScV16Sc", "")
 BUILTIN(__builtin_altivec_vaddubs, "V16UcV16UcV16Uc", "")

clang/lib/Basic/Targets/PPC.cpp

static void defineXLCompatMacros(MacroBuilder &Builder) {
   Builder.defineMacro("__fetch_and_add", "__builtin_ppc_fetch_and_add");
   Builder.defineMacro("__fetch_and_addlp", "__builtin_ppc_fetch_and_addlp");
   Builder.defineMacro("__fetch_and_and", "__builtin_ppc_fetch_and_and");
   Builder.defineMacro("__fetch_and_andlp", "__builtin_ppc_fetch_and_andlp");
   Builder.defineMacro("__fetch_and_or", "__builtin_ppc_fetch_and_or");
   Builder.defineMacro("__fetch_and_orlp", "__builtin_ppc_fetch_and_orlp");
   Builder.defineMacro("__fetch_and_swap", "__builtin_ppc_fetch_and_swap");
   Builder.defineMacro("__fetch_and_swaplp", "__builtin_ppc_fetch_and_swaplp");
+  Builder.defineMacro("__ldarx", "__builtin_ppc_ldarx");
+  Builder.defineMacro("__lwarx", "__builtin_ppc_lwarx");
+  Builder.defineMacro("__stdcx", "__builtin_ppc_stdcx");
+  Builder.defineMacro("__stwcx", "__builtin_ppc_stwcx");
 }
 
 /// PPCTargetInfo::getTargetDefines - Return a set of the PowerPC-specific
 /// #defines that are not tied to a specific subtarget.
 void PPCTargetInfo::getTargetDefines(const LangOptions &Opts,
                                      MacroBuilder &Builder) const {
 
   defineXLCompatMacros(Builder);

clang/lib/Sema/SemaChecking.cpp

 }
 
 static bool isPPC_64Builtin(unsigned BuiltinID) {
   // These builtins only work on PPC 64bit targets.
   switch (BuiltinID) {
   case PPC::BI__builtin_divde:
   case PPC::BI__builtin_divdeu:
   case PPC::BI__builtin_bpermd:
+  case PPC::BI__builtin_ppc_ldarx:
+  case PPC::BI__builtin_ppc_stdcx:
     return true;
   }
   return false;
 }
 
 static bool SemaFeatureCheck(Sema &S, CallExpr *TheCall,
                              StringRef FeatureToCheck, unsigned DiagID) {
   if (!S.Context.getTargetInfo().hasFeature(FeatureToCheck))

clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c

+// RUN: not %clang_cc1 -triple=powerpc-unknown-aix -emit-llvm %s -o - 2>&1 |\
+// RUN: FileCheck %s --check-prefix=CHECK32-ERROR
+// RUN: %clang_cc1 -triple=powerpc64-unknown-aix -emit-llvm %s -o - | \
+// RUN: FileCheck %s --check-prefix=CHECK64
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-unknown -emit-llvm %s \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK64
+// RUN: %clang_cc1 -triple=powerpc64-unknown-unknown -emit-llvm %s \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK64
+
+long test_ldarx(volatile long* a) {
+  // CHECK64-LABEL: @test_ldarx
+  // CHECK64: %0 = load i64*, i64** %a.addr, align 8
+  // CHECK64: %1 = bitcast i64* %0 to i8*
+  // CHECK64: %2 = call i64 @llvm.ppc.ldarx(i8* %1)
+  // CHECK32-ERROR: error: this builtin is only available on 64-bit targets
+  return __ldarx(a);
+}
+
+int test_stdcx(volatile long* addr, long val) {
+  // CHECK64-LABEL: @test_stdcx
+  // CHECK64: %0 = load i64*, i64** %addr.addr, align 8
+  // CHECK64: %1 = bitcast i64* %0 to i8*
+  // CHECK64: %2 = load i64, i64* %val.addr, align 8
+  // CHECK64: %3 = call i32 @llvm.ppc.stdcx(i8* %1, i64 %2)
+  // CHECK32-ERROR: error: this builtin is only available on 64-bit targets
+  return __stdcx(addr, val);
+}

clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c

+// RUN: %clang_cc1 -triple=powerpc-unknown-aix -emit-llvm %s -o - | \
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple=powerpc64-unknown-aix -emit-llvm %s -o - | \
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-unknown -emit-llvm %s \
+// RUN:  -o - | FileCheck %s
+// RUN: %clang_cc1 -triple=powerpc64-unknown-unknown -emit-llvm %s \
+// RUN:  -o - | FileCheck %s
+
+int test_lwarx(volatile int* a) {
+  // CHECK: @test_lwarx
+  // CHECK: %1 = bitcast i32* %0 to i8*
+  // CHECK: %2 = call i32 @llvm.ppc.lwarx(i8* %1)
+  return __lwarx(a);
+}
+int test_stwcx(volatile int* a, int val) {
+  // CHECK: @test_stwcx
+  // CHECK: %1 = bitcast i32* %0 to i8*
+  // CHECK: %2 = load i32, i32* %val.addr, align 4
+  // CHECK: %3 = call i32 @llvm.ppc.stwcx(i8* %1, i32 %2)
+  return __stwcx(a, val);
+}

clang/test/CodeGen/builtins-ppc-xlcompat-error.c

+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -fsyntax-only \
+// RUN:   -Wall -Werror -verify %s
+// RUN: %clang_cc1 -triple powerpc64le-unknown-unknown -fsyntax-only \
+// RUN:   -Wall -Werror -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -fsyntax-only \
+// RUN:   -Wall -Werror -verify %s
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -fsyntax-only \
+// RUN:   -Wall -Werror -verify %s
+
+long long lla, llb;
+int ia, ib;
+
+void test_trap(void) {
+  __tdw(lla, llb, 50); //expected-error {{argument value 50 is outside the valid range [0, 31]}}
+  __tw(ia, ib, 50); //expected-error {{argument value 50 is outside the valid range [0, 31]}}
+}

llvm/include/llvm/IR/IntrinsicsPowerPC.td

def int_ppc_lwsync : GCCBuiltin<"__builtin_ppc_lwsync">,
                        Intrinsic<[], [], []>;
   def int_ppc_iospace_lwsync : GCCBuiltin<"__builtin_ppc_iospace_lwsync">,
                                Intrinsic<[], [], []>;
   // eieio instruction
   def int_ppc_eieio : GCCBuiltin<"__builtin_ppc_eieio">,
                       Intrinsic<[],[],[]>;
   def int_ppc_iospace_eieio : GCCBuiltin<"__builtin_ppc_iospace_eieio">,
                               Intrinsic<[],[],[]>;
+  def int_ppc_stdcx : GCCBuiltin<"__builtin_ppc_stdcx">,
+                      Intrinsic<[llvm_i32_ty], [llvm_ptr_ty, llvm_i64_ty],
+                                [IntrWriteMem]>;
+  def int_ppc_stwcx : GCCBuiltin<"__builtin_ppc_stwcx">,
+                      Intrinsic<[llvm_i32_ty], [llvm_ptr_ty, llvm_i32_ty],
+                                [IntrWriteMem]>;
+  def int_ppc_lwarx : GCCBuiltin<"__builtin_ppc_lwarx">,
+                      Intrinsic<[llvm_i32_ty], [llvm_ptr_ty], [IntrNoMem]>;
+  def int_ppc_ldarx : GCCBuiltin<"__builtin_ppc_ldarx">,
+                      Intrinsic<[llvm_i64_ty], [llvm_ptr_ty], [IntrNoMem]>;
 }
 

llvm/lib/Target/PowerPC/PPCInstr64Bit.td

 
 // SLB Invalidate Entry Global
 def SLBIEG : XForm_26<31, 466, (outs), (ins gprc:$RS, gprc:$RB),
                       "slbieg $RS, $RB", IIC_SprSLBIEG, []>;
 // SLB Synchronize
 def SLBSYNC : XForm_0<31, 338, (outs), (ins), "slbsync", IIC_SprSLBSYNC, []>;
 
 } // IsISA3_0
+
+def : Pat<(int_ppc_stdcx ForceXForm:$dst, g8rc:$A),
+          (STDCX g8rc:$A, ForceXForm:$dst)>;
+def : Pat<(int_ppc_ldarx ForceXForm:$dst),
+          (LDARX ForceXForm:$dst)>;

llvm/lib/Target/PowerPC/PPCInstrInfo.td

def DWBytes3210 {
   dag DWord =
     (i64 (INSERT_SUBREG (i64 (IMPLICIT_DEF)), Word, sub_32));
 }
 
 // Now both high word and low word are reversed, next
 // swap the high word and low word.
 def : Pat<(i64 (bitreverse i64:$A)),
   (OR8 (RLDICR DWBytes7654.DWord, 32, 31), DWBytes3210.DWord)>;
+
+def : Pat<(int_ppc_lwarx ForceXForm:$dst),
+          (LWARX ForceXForm:$dst)>;
+def : Pat<(int_ppc_stwcx ForceXForm:$dst, gprc:$A),
+          (STWCX gprc:$A, ForceXForm:$dst)>;

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond-64bit-only.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr8 < %s | FileCheck %s --check-prefix=CHECK
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:   -mcpu=pwr8 < %s | FileCheck %s --check-prefix=CHECK
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN:   -mcpu=pwr8 < %s | FileCheck %s --check-prefix=CHECK
+
+declare i64 @llvm.ppc.ldarx(i8*)
+define dso_local i64 @test_ldarx(i64* readnone %a) {
+; CHECK-LABEL: test_ldarx:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    ldarx 3, 0, 3
+; CHECK-NEXT:    blr
+entry:
+  %0 = bitcast i64* %a to i8*
+  %1 = tail call i64 @llvm.ppc.ldarx(i8* %0)
+  ret i64 %1
+}
+
+declare i32 @llvm.ppc.stdcx(i8*, i64)
+define dso_local i64 @test(i64* %a, i64 %b) {
+; CHECK-LABEL: test:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    stdcx. 4, 0, 3
+; CHECK-NEXT:    mfocrf 3, 128
+; CHECK-NEXT:    srwi 3, 3, 28
+; CHECK-NEXT:    extsw 3, 3
+; CHECK-NEXT:    blr
+entry:
+  %0 = bitcast i64* %a to i8*
+  %1 = tail call i32 @llvm.ppc.stdcx(i8* %0, i64 %b)
+  %conv = sext i32 %1 to i64
+  ret i64 %conv
+}

llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReserve-StoreCond.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr8 < %s | FileCheck %s --check-prefix=CHECK-64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:   -mcpu=pwr8 < %s | FileCheck %s --check-prefix=CHECK-64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix \
+; RUN:   -mcpu=pwr8 < %s | FileCheck %s --check-prefix=CHECK-32
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN:   -mcpu=pwr8 < %s | FileCheck %s --check-prefix=CHECK-64
+
+declare i32 @llvm.ppc.lwarx(i8*)
+define dso_local signext i32 @test_lwarx(i32* readnone %a) {
+; CHECK-64-LABEL: test_lwarx:
+; CHECK-64:       # %bb.0: # %entry
+; CHECK-64-NEXT:    lwarx 3, 0, 3
+; CHECK-64-NEXT:    extsw 3, 3
+; CHECK-64-NEXT:    blr
+;
+; CHECK-32-LABEL: test_lwarx:
+; CHECK-32:       # %bb.0: # %entry
+; CHECK-32-NEXT:    lwarx 3, 0, 3
+; CHECK-32-NEXT:    blr
+entry:
+  %0 = bitcast i32* %a to i8*
+  %1 = tail call i32 @llvm.ppc.lwarx(i8* %0)
+  ret i32 %1
+}
+
+declare i32 @llvm.ppc.stwcx(i8*, i32)
+define dso_local signext i32 @test_stwcx(i32* %a, i32 signext %b) {
+; CHECK-64-LABEL: test_stwcx:
+; CHECK-64:       # %bb.0: # %entry
+; CHECK-64-NEXT:    stwcx. 4, 0, 3
+; CHECK-64-NEXT:    mfocrf 3, 128
+; CHECK-64-NEXT:    srwi 3, 3, 28
+; CHECK-64-NEXT:    extsw 3, 3
+; CHECK-64-NEXT:    blr
+;
+; CHECK-32-LABEL: test_stwcx:
+; CHECK-32:       # %bb.0: # %entry
+; CHECK-32-NEXT:    stwcx. 4, 0, 3
+; CHECK-32-NEXT:    mfocrf 3, 128
+; CHECK-32-NEXT:    srwi 3, 3, 28
+; CHECK-32-NEXT:    blr
+entry:
+  %0 = bitcast i32* %a to i8*
+  %1 = tail call i32 @llvm.ppc.stwcx(i8* %0, i32 %b)
+  ret i32 %1
+}