fmayer added inline comments.
================ Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:410 + Dummy.setArch(Triple::aarch64); + if (shouldUseStackSafetyAnalysis(Dummy, DisableOptimization)) + AU.addRequired<StackSafetyGlobalInfoWrapperPass>(); ---------------- fmayer wrote: > vitalybuka wrote: > > Why not just: > > ``` > > if (!DisableOptimization || ClUseStackSafety) > > AU.addRequired<StackSafetyGlobalInfoWrapperPass>(); > > ``` > Because `ClUseStackSafety` defaults to true, so this will be quite useless > except if the user explicitly sets it to false. So then you end up > duplicating the logic in `shouldUseStackSafetyAnalysis`, which is why I went > for the Dummy (thinking about it, we don't actually need to setArch on it). How about this (mightUseStackSafetyAnalysis)? This seems like a nice tradeoff without duplication of the logic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105703/new/ https://reviews.llvm.org/D105703 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
