[PATCH] D104386: [PowerPC][Builtins] Added a number of builtins for compatibility with XL.

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

Can the test cases that just check for specific IR being produced be merged together? Seems unnecessary to have all of these separate test cases.
Perhaps something along the lines of:

- Test case for diagnostics of invalid use
- Test case for produced IR
- Test case for specific metadata being produced



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9732
   "argument should be an 8-bit value shifted by a multiple of 8 bits, or in the form 0x??FF">;
+def err_argument_not_contiguous_bit_field : Error<
+  "argument %0 value should represent a contiguous bit field">;
----------------
I think this comes from another patch that is up for review. You should base this patch on top of that patch and mark the review as a dependency. It makes the review easier if the review only contains code that is meant to go in this commit.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15257-15258
+    const Expr *Ptr = E->getArg(1);
+    Value *PtrValue = EmitScalarExpr(Ptr);
+    Value *AlignmentValue = EmitScalarExpr(E->getArg(0));
+    ConstantInt *AlignmentCI = cast<ConstantInt>(AlignmentValue);
----------------
Are these two just `Ops[0], Ops[1]`?


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15270-15271
+  case PPC::BI__builtin_ppc_rdlam: {
+    Value *Src = EmitScalarExpr(E->getArg(0));
+    Value *ShiftAmt = EmitScalarExpr(E->getArg(1));
+
----------------
Same comment as above.


================
Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-expect.c:1
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown \
----------------
Should this not test for the meta data that `__builtin_expect` produces?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104386