lei requested changes to this revision.
lei added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang/include/clang/Sema/Sema.h:12680
bool CheckPPCMMAType(QualType Type, SourceLocation TypeLoc);
+ bool CheckPPCTestDataClassType(CallExpr *TheCall);
----------------
need to remove.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16039-16040
+ case PPC::BI__builtin_ppc_test_data_class:
+ Value *ArgValue = EmitScalarExpr(E->getArg(0));
+ llvm::Type *ArgType = ArgValue->getType();
+ unsigned Int;
----------------
`ArgValue` is only used one so not needed.
```
llvm::Type *ArgType = EmitScalarExpr(E->getArg(0))->getType();
```
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16041
+ llvm::Type *ArgType = ArgValue->getType();
+ unsigned Int;
+ if (ArgType->isDoubleTy()) {
----------------
variables should be discriptive of what they represent. This is no diff then a
single char variable 🙂
```
unsigned IntrinsicID;
```
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16042
+ unsigned Int;
+ if (ArgType->isDoubleTy()) {
+ Int = Intrinsic::ppc_test_data_class_d;
----------------
braces here are redundant.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16050
+ Function *F = CGM.getIntrinsic(Int);
+ return Builder.CreateCall(F, Ops, "test_data_class");
}
----------------
Try to refrain from def one-time use variables.
```
return Builder.CreateCall(CGM.getIntrinsic(Int), Ops, "test_data_class");
```
================
Comment at: clang/lib/Sema/SemaChecking.cpp:3492
+ Diag(TheCall->getBeginLoc(), diag::err_ppc_invalid_test_data_class_type);
+ ArgTypeIsInvalid = true;
+ }
----------------
I'm not sure this is needed... can't we just `return true` here since this is a
`S` error?
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10382-10383
+ switch (IntrinsicID) {
+ default:
+ llvm_unreachable("Unknown Intrinsic");
+ case Intrinsic::ppc_compare_exp_lt:
----------------
I dont' think this is needed since you will only be here if he IntrinsicID
matches the lines listed prior to this block.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10397-10398
+ }
+ SDValue Op1 = Op.getOperand(1);
+ SDValue Op2 = Op.getOperand(2);
+ SDValue Ops[]{
----------------
one time used variables can be removed.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10408-10417
+ switch (IntrinsicID) {
+ default:
+ llvm_unreachable("Unknown Intrinsic");
+ case Intrinsic::ppc_test_data_class_d:
+ CmprOpc = PPC::XSTSTDCDP;
+ break;
+ case Intrinsic::ppc_test_data_class_f:
----------------
this can just be an if/else since you won't be in this block unless the
IntrisicID are `ppc_test_data_class_[d|f]`
```
unsigned CmprOpc = PPC::XSTSTDCDP;
if (IntrinsicID == Intrinsic::ppc_test_data_class_f)
CmprOpc = PPC::XSTSTDCSP;
```
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10419
+ SDValue Op1 = Op.getOperand(2);
+ SDValue Op2 = Op.getOperand(1);
+ SDValue Ops[]{
----------------
one-time use variable. Can be merged into the call below.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109437/new/
https://reviews.llvm.org/D109437
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
