shchenz added a comment. Please make sure the patch is able to compile first. I fixed two build failures, it still fails : (
================ Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:221 + def int_ppc_kill_canary + : GCCBuiltin<"__builtin_ppc_kill_canary">, + Intrinsic<[], ---------------- Need rebase your code. `GCCBuiltin` is not a valid class anymore. See https://reviews.llvm.org/D127460 ================ Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5013 auto IntrinsicID = N->getConstantOperandVal(1); + if (IntrinsicID == Intrinsic::ppc_tdw || IntrinsicID == Intrinsic::ppc_tw) { ---------------- Nit: avoid this unnecessary change ================ Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:61 #include "llvm/CodeGen/ValueTypes.h" +#include "llvm/CodeGen/GlobalISel/IRTranslator.h" #include "llvm/IR/CallingConv.h" ---------------- Do we need this header? A little strange... ================ Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11148 + EVT VT = DAG.getTargetLoweringInfo().getValueType(DAG.getDataLayout(), GV->getType(), true); + SDValue canaryLoc = DAG.getGlobalAddress(GV, DL, VT); + ---------------- Please make sure when you post the patch, it is ok to compile. `DL` seems undefined, we use `dl` here. Also please read the coding style of llvm first. https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly `canaryLoc` does not follow the style, should be `CanaryLoc`. ================ Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11158 + canaryLoc, + MachinePointerInfo() + ); ---------------- All these new added codes need clang-format ================ Comment at: llvm/test/CodeGen/PowerPC/kill-canary-intrinsic-aix.ll:5 + +declare void @llvm.ppc.kill.canary() +define dso_local void @test_kill_canary() { ---------------- We can merge these two files `kill-canary-intrinsic-aix.ll` and `kill-canary-intrinsic-linux.ll` into one file with different runlines but with different check prefix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128652/new/ https://reviews.llvm.org/D128652 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
