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

[ChenZheng via Phabricator via llvm-commits]

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