mehdi_amini accepted this revision.
mehdi_amini added a comment.
This revision is now accepted and ready to land.
LGTM! Exciting to see the new PM arriving in clang :)
================
Comment at: lib/CodeGen/BackendUtil.cpp:757
+ setCommandLineOpts();
+ cl::PrintOptionValues();
+
----------------
This is different from the legacy path, why?
================
Comment at: lib/CodeGen/BackendUtil.cpp:762
+ CreateTargetMachine(/*MustCreateTM*/ true);
+ TheModule->setDataLayout(TM->createDataLayout());
+
----------------
The legacy path accounts for the possibility of failing to create the TM
```
CreateTargetMachine(UsesCodeGen);
if (UsesCodeGen && !TM)
return;
```
================
Comment at: lib/CodeGen/BackendUtil.cpp:831
+ // Now that we have all of the passes ready, run them.
+ MPM.run(*TheModule, MAM);
+
----------------
No need to wrap in a block with a `PrettyStackTraceString` like the legacy PM?
================
Comment at: test/Driver/clang_f_opts.c:475
+// RUN: %clang -### -fexperimental-new-pass-manager
-fno-experimental-new-pass-manager %s 2>&1 | FileCheck --check-prefix=CHECK
--check-prefix=CHECK-NO-NEW-PM %s
+// CHECK-NOT: argument unused
+// CHECK-NEW-PM: -fexperimental-new-pass-manager
----------------
Not critical, but using the "CHECK" prefix isn't nice in a file intended to be
shared for multiple tests.
https://reviews.llvm.org/D28077
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
