[PATCH] D105236: [PowerPC] Implament Load and Reserve and Store Conditional Builtins

[Victor Huang via Phabricator via cfe-commits]

NeHuang added inline comments.


================
Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c:1
+// RUN: not %clang_cc1 -triple=powerpc-unknown-aix -O2 -S -emit-llvm %s -o - 2>&1 |\
+// RUN: FileCheck %s --check-prefix=CHECK32-ERROR
----------------
`-S` seems redundant.
Any reason we need `-O2` for these tests?


================
Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c:1
+// RUN: %clang_cc1 -triple=powerpc-unknown-aix -O2 -S -emit-llvm %s -o - | \
+// RUN: FileCheck %s
----------------
same as above.


================
Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-LoadReseve-StoreCond.c:11
+int test_lwarx(volatile int* a) {
+  // CHECK: entry:
+  // CHECK: %0 = bitcast i32* %a to i8*
----------------
stefanp wrote:
> 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. 
Agreed. We should first do `CHECK-LABEL` with the function name.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105236/new/

https://reviews.llvm.org/D105236