nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.
Mostly minor comments, but it'll be good to have another look to make sure
they're all addressed.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15101
+ // ret i64 %or
+ else {
+ Function *F =
----------------
Nit: no `else` after `return`.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15104
+ CGM.getIntrinsic(Intrinsic::ppc_cmpb, {Int32Ty, Int32Ty, Int32Ty});
+ // %conv = trunc i64 %a to i32
+ Value *A = Builder.CreateTrunc(Ops[0], Int32Ty);
----------------
These comments are superfluous. You have the entire sequence listed above, no
need to repeat it piece-wise here.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:15105
+ // %conv = trunc i64 %a to i32
+ Value *A = Builder.CreateTrunc(Ops[0], Int32Ty);
+ // %conv1 = trunc i64 %b to i32
----------------
Please do not name variables with non-descriptive single letters. It is
somewhat conventional to use suffixes such as `Hi/Lo` for high/low order parts
of a value.
================
Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:1537
+ def int_ppc_cmpb
+ : Intrinsic<[llvm_anyint_ty], [llvm_anyint_ty, llvm_anyint_ty],
[IntrNoMem]>;
// multiply
----------------
Line too long?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105194/new/
https://reviews.llvm.org/D105194
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
