vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1511
 
+  if (ClUseStackSafety && SSGI != nullptr && findAllocaForValue(Ptr) &&
+      SSGI->stackAccessIsSafe(*Inst))
----------------
check is not needed, we SSGI should be already null in this case


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1499-1502
+  if (ClUseStackSafety && findAllocaForValue(Ptr)) {
+    if (SSI && SSI->stackAccessIsSafe(*Inst)) {
+      return true;
+    }
----------------
fmayer wrote:
> Doesn't need nested if.
> 
> I would order this as ClUseStackSafety && SSI && findAllocaForValue(Ptr) && 
> ... (cheapest first).
findAllocaForValue is DFS (expensive), stackAccessIsSafe is hash table lookup 
(cheep)
so I would recommend to switch them


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112098/new/

https://reviews.llvm.org/D112098

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to