[PATCH] D77249: [MSan] Pass command line options to MSan with new pass manager

Leonard Chan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 3 11:53:41 PDT 2020


leonardchan 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 && \
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77249





More information about the cfe-commits mailing list