glider accepted this revision.
glider added a comment.
This revision is now accepted and ready to land.

LGTM assuming allyesconfig builds.



================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:759
         // do globals-gc.
-        UseCtorComdat(UseGlobalsGC && ClWithComdat) {
-    this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
-    this->CompileKernel =
-        ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan : CompileKernel;
-
+        UseCtorComdat(UseGlobalsGC && ClWithComdat && !this->CompileKernel) {
     C = &(M.getContext());
----------------
melver wrote:
> glider wrote:
> > `UseGlobalsGC` assumes `!this->CompileKernel` already, doesn't it?
> I experimented a bit and found that even if one of them is enabled things 
> break (say if you remove UseGlobalsGC here). This is more explicit and less 
> error-prone in case somebody removes UseGlobalsGC from UseCtorComdat. And 
> there is no real penalty here for having it on both.
That's strange, but ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81390



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

Reply via email to