rnk added inline comments.
================ Comment at: compiler-rt/test/msan/chained_origin_empty_stack_npm.cpp:4 +// this test. +// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 \ +// RUN: -fexperimental-new-pass-manager -O3 %s -o %t && \ ---------------- leonardchan wrote: > nemanjai wrote: > > nemanjai wrote: > > > vitalybuka wrote: > > > > Why not to add RUN: section with -fexperimental-new-pass-manager into > > > > original tests? > > > I just felt that this is a simpler way forward for a couple of reasons: > > > 1. Once the default switches, it is a very obvious change to just delete > > > these files rather than digging through the code inside the existing ones > > > 2. Many of the tests actually contain the testing that is split up into > > > multiple steps so I would have to duplicate all the steps for the NPM vs. > > > default builds: > > > - compile/link > > > - run with one option set and FileCheck > > > - run with another option set and FileCheck > > > - rinse/repeat > > > (example: chained_origin_limits.cpp) > > > > > > But of course, if there are strong objections to this approach, I can > > > certainly go the other way. > > Seems Phabricator reformatted what I wrote here. Points 3, 4, 5, 6 were > > supposed to be sub-bullets for 2. > > Basically, I tried to describe that in the mentioned test case, I would > > have to replicate a number of subsequent steps for each `RUN` directive > > that invokes the compiler. > If we're going this way, I think the original tests should explicitly have > `-fno-experimental-new-pass-manager`. Also no strong preference towards > either way. I don't think we should even make changes to the tests in compiler-rt. We should write a targeted test in clang/test/CodeGen that ensures these options are passed down correctly to the MSan instrumentation pass. It should be easy to FileCheck the IR to look for the appropriate instrumentation callbacks. We can run that test with the new and old PM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77249/new/ https://reviews.llvm.org/D77249 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
