[PATCH] D18019: [PM] Port GVN to the new pass manager, wire it up, and teach a couple of tests to run GVN in both modes.

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 11 10:34:25 PST 2016


chandlerc added inline comments.

================
Comment at: llvm/trunk/lib/Target/NVPTX/NVPTXTargetMachine.cpp:47
@@ -46,2 +46,3 @@
 #include "llvm/Transforms/Scalar.h"
+#include "llvm/Transforms/Scalar/GVN.h"
 
----------------
davidxl wrote:
> chandlerc wrote:
> > davidxl wrote:
> > > I noticed this line -- why is this inclusion needed?
> > Because NVPTX adds GVN into its target-specific code generation pass pipeline, and so it needs the 'createGVNPass' function. (You should be able to delete this and see the compile fail. At least, that's how I knew to add it.)
> The question is why Scalar .h does not include GVN.h?
Historically, LLVM used monolithic headers and functions that returned opaque types for passes.

A large chunk of the new pass manager design moves us towards "normal" patterns. There is a header, with a type, and maybe some helper functions. It allows much more fine grained headers so folks only pull in what they need. And with new-style passes it allows passes to be used outside the context of a pass manager if desirable (for unit testing for example).

If Scalar.h included GVN.h, then it would pull in a pretty large pile of code that doesn't make sense for everyone to pull in.

If Scalar.h continued to own the createGVNPass function, it would break proper modularity, as the constructs being defined in GVN.cpp would be declared in different and fairly distant places.


Repository:
  rL LLVM

http://reviews.llvm.org/D18019





More information about the llvm-commits mailing list