NeHuang requested changes to this revision.
NeHuang added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang/include/clang/Basic/BuiltinsPPC.def:32
-// builtins for compatibility with the XL compiler
+// XL Compatibility built-ins
BUILTIN(__builtin_ppc_popcntb, "ULiULi", "")
----------------
seems like rebase issue that comments got overwritten.
================
Comment at: clang/include/clang/Basic/BuiltinsPPC.def:50
BUILTIN(__builtin_ppc_compare_and_swaplp, "iLiD*Li*Li", "")
+BUILTIN(__builtin_ppc_tdw, "vLLiLLiIi", "")
+BUILTIN(__builtin_ppc_tw, "viiIi", "")
----------------
definition here not matching prototype in document
```
void __tdw ( long a, long b, unsigned int TO);
```
================
Comment at: clang/include/clang/Basic/BuiltinsPPC.def:51
+BUILTIN(__builtin_ppc_tdw, "vLLiLLiIi", "")
+BUILTIN(__builtin_ppc_tw, "viiIi", "")
+BUILTIN(__builtin_ppc_trap, "vi", "")
----------------
prototype
```
void __tw (int a, int b, unsigned int TO);
```
why not using `Ui` for the last arg?
================
Comment at: clang/lib/Sema/SemaChecking.cpp:3347
+ case PPC::BI__builtin_ppc_tw:
+ return SemaBuiltinConstantArgRange(TheCall, 2, 1, 31);
+ case PPC::BI__builtin_ppc_tdw:
----------------
range suppose to be 0 to 31 based on document.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:3349
+ case PPC::BI__builtin_ppc_tdw:
+ return SemaBuiltinConstantArgRange(TheCall, 2, 1, 31);
#define CUSTOM_BUILTIN(Name, Intr, Types, Acc) \
----------------
same as above.
================
Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-conversionfunc.c:2
+// RUN: %clang_cc1 -O2 -triple powerpc64-unknown-unknown \
+// RUN: -emit-llvm %s -o - -target-cpu pwr9 | FileCheck %s
+// RUN: %clang_cc1 -O2 -triple powerpc64le-unknown-unknown \
----------------
are these builtins all pwr9 only?
- If yes, please rename the file.
- If not, please use pwr8 for LE test and pwr7 for BE cases.
================
Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-conversionfunc.c:9
+// RUN: -emit-llvm %s -o - -target-cpu pwr9 | FileCheck %s
+
+double test_fcfid(double a) {
----------------
you can define extern variables here for the bulitins.
================
Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-error.c:16
+#ifdef __PPC64__
+ __tdw(lla, llb, 50); //expected-error {{argument value 50 is outside the
valid range [1, 31]}}
+#endif
----------------
range should be 0 to 31 as described in document.
```
TO A value of 0 to 31 inclusive. Each bit position, if set, indicates one or
more of
the following possible conditions:
```
================
Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-error.c:18
+#endif
+ __tw(ia, ib, 50); //expected-error {{argument value 50 is outside the valid
range [1, 31]}}
+}
----------------
same as above
================
Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-trap.c:2
+// RUN: %clang_cc1 -O2 -triple powerpc64-unknown-unknown \
+// RUN: -emit-llvm %s -o - -target-cpu pwr9 | \
+// RUN: FileCheck %s --check-prefixes=CHECK64,CHECK
----------------
same as above.
================
Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-conversionfunc.ll:3
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN: -mcpu=pwr9 < %s | FileCheck %s --check-prefix=CHECK-64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
----------------
32 bit and 64 bit results look identical, you do not need prefixes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103668/new/
https://reviews.llvm.org/D103668
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
