[PATCH] D18421: [WIP] MachineFunction Properties

[Quentin Colombet via llvm-commits]

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

Hi Derek,

Simple and efficiency, nice :).

LGTM.

I agree that, as it is, the checks are enough, we do not need to expose this to the pass manager.

Couple of nits inlined.

Cheers,
-Quentin


================
Comment at: include/llvm/CodeGen/MachineFunction.h:95
@@ +94,3 @@
+// Each of these has checking code in the MachineVerifier, and passes can
+// require that a property be set.
+class MachineFunctionProperties {
----------------
“Promote” the comment to doxygen style (///).

================
Comment at: include/llvm/CodeGen/MachineFunction.h:98
@@ +97,3 @@
+  BitVector Properties =
+      BitVector(static_cast<unsigned>(Property::LastProperty));
+  // Stated in "positive" form; i.e. a pass could require that the property
----------------
Just a remark: if we want to give more control to the targets with possible properties, we can make the universe of the BitVector an argument of the constructor and just assert that the universe supports at least LastProperty elements.

================
Comment at: include/llvm/CodeGen/MachineFunction.h:210
@@ -156,1 +209,3 @@
 
+  MachineFunctionProperties Properties;
+
----------------
Add a comment.

================
Comment at: lib/CodeGen/MachineFunctionPass.cpp:50
@@ +49,3 @@
+
+  assert(MFProps.verifyRequiredProperties(RequiredProperties));
+
----------------
Add a message in the assert.

================
Comment at: lib/Target/NVPTX/NVPTXTargetMachine.cpp:229
@@ -226,1 +228,3 @@
+  disablePass(&PostRASchedulerID);
+  disablePass(&FuncletLayoutID);
 
----------------
Great!
It means it already pays off by simplifying some compilation pipelines :).


http://reviews.llvm.org/D18421