nemanjai added inline comments.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10702
+
+ // If SafeStack or !StackProtector, kill_canary is not supported.
+ if (MF.getFunction().hasFnAttribute(Attribute::SafeStack) ||
----------------
Again, this comment is nothing more than a re-reading of the code. As such, it
is not useful. The comment should say what is happening and why. Something
along the lines of:
```
// The kill_canary intrinsic only makes sense when the Stack Protector
// feature is on in the function. It can also not be used in conjunction
// with safe stack because the latter splits the stack and the canary
// value isn't used (i.e. safe stack supersedes stack protector).
// In situations where the kill_canary intrinsic is not supported,
// we simply replace uses of its chain with its input chain, causing
// the SDAG CSE to remove the node.
```
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10720-10749
+ if (useLoadStackGuardNode()) {
+ MachineSDNode *LSG =
+ DAG.getMachineNode(PPC::LOAD_STACK_GUARD, DL, VT, Op->getOperand(0));
+ Load = SDValue(LSG, 0);
+
+ // Frame index used to determine stack guard location if
+ // LOAD_STACK_GUARD is used.
----------------
The common code should just be common. Seems like the only thing that changes
is how we load the value. Please refactor this to something like:
```
if (useLoadStackGuardNode()) {
...
Load = ... // stack guard load
} else if (Value *GV = getSDagStackGuard(*M)) {
...
Load = ... // canary word global load
} else
llvm_unreachable("Unhandled stack guard case");
Store = ... // common store
return Store; // (or you can just create the store in-line and return it
directly)
```
================
Comment at: llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix
-ppc-vsr-nums-as-vr \
+; RUN: -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s
-check-prefix=CHECK-AIX
----------------
Please add one RUN line with `-O0` to ensure that this works as expected with
Fast ISel.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129016/new/
https://reviews.llvm.org/D129016
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
