[llvm-commits] [PATCH] Invariants (and Assume Aligned) - LLVM

[Dmitri Gribenko]

================
Comment at: docs/LangRef.html:9083-9085
@@ +9082,5 @@
+
+<p>The intrinsic allows the optimizer to assume that the provided condition is
+   always true. No code is generated for this intrinsic, and instructions that
+   contribute only to the provided condition are not used for code generation.</p>
+</div>
----------------
What about adding explicitly: "If the condition does not hold during execution, the behavior is undefined"?

================
Comment at: include/llvm/Analysis/EphemeralValues.h:9-11
@@ +8,5 @@
+//===----------------------------------------------------------------------===//
+//
+// Calculate ephemeral values - those used only (indirectly) by invariants.
+//
+//===----------------------------------------------------------------------===//
----------------
Three slashes and '\file', please.


================
Comment at: include/llvm/Analysis/EphemeralValues.h:35
@@ +34,3 @@
+  // Returns true if the provided value is ephemeral.
+  bool isEphemeralValue(Value *V) const {
+    return EphValues.count(V);
----------------
Three slashes please.

================
Comment at: include/llvm/Analysis/InlineCost.h:110
@@ -108,1 +109,3 @@
     const DataLayout *TD;
+    // EphemeralValues if available, or null.
+    const EphemeralValues *EV;
----------------
Three slashes...

================
Comment at: lib/Analysis/CaptureTracking.cpp:139-142
@@ -125,19 +138,6 @@
     }
-    case Instruction::Load:
-      // Loading from a pointer does not cause it to be captured.
-      break;
-    case Instruction::VAArg:
-      // "va-arg" from a pointer does not cause it to be captured.
-      break;
-    case Instruction::Store:
-      if (V == I->getOperand(0))
-        // Stored the pointer - conservatively assume it may be captured.
-        if (Tracker->captured(U))
-          return;
-      // Storing to the pointee does not cause the pointer to be captured.
-      break;
     case Instruction::BitCast:
     case Instruction::GetElementPtr:
     case Instruction::PHI:
     case Instruction::Select:
       // The original value is not captured via this if the new value isn't.
----------------
Did you really intend to just move this piece of code a few lines down?

================
Comment at: lib/Analysis/EphemeralValues.cpp:10-12
@@ +9,5 @@
+//===----------------------------------------------------------------------===//
+//
+// This file implements ephemeral value determination.
+//
+//===----------------------------------------------------------------------===//
----------------
Three slashes and '\file', please.

================
Comment at: include/llvm/Analysis/EphemeralValues.h:44
@@ +43,3 @@
+  static char ID;
+  explicit EphemeralValues();
+
----------------
Does this need 'explicit'?

================
Comment at: lib/Analysis/InlineCost.cpp:47
@@ -45,1 +46,3 @@
   const DataLayout *const TD;
+  // EphemeralValues if available, or null.
+  const EphemeralValues *const EV;
----------------
Three slashes, please.

================
Comment at: lib/CodeGen/IntrinsicLowering.cpp:458
@@ -456,3 +457,3 @@
   case Intrinsic::var_annotation:
     break;   // Strip out annotate intrinsic
     
----------------
Please correct the comment (something like "strip out these intrinsics")

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5023
@@ -5021,3 +5022,3 @@
   case Intrinsic::var_annotation:
     // Discard annotate attributes
     return 0;
----------------
Please correct the comment.

================
Comment at: lib/Transforms/Scalar/AlignmentInvProp.cpp:10-12
@@ +9,5 @@
+//===----------------------------------------------------------------------===//
+//
+// This file implements alignment invariant propagation.
+//
+//===----------------------------------------------------------------------===//
----------------
Three slashes and '\file'

================
Comment at: lib/Transforms/Scalar/AlignmentInvProp.cpp:243
@@ +242,3 @@
+      unsigned(sizeof(unsigned) * CHAR_BIT - 1));
+    Alignment = std::min(1u << TrailingOnes, +Value::MaximumAlignment);
+
----------------
Is the '+' required here?


http://llvm-reviews.chandlerc.com/D150