[PATCH] D128652: [PowerPC] Finished kill_canary implementation and debugging

ChenZheng via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 30 07:22:54 PDT 2022

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.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list