LLVM support for IR invariants (and alignment assumptions as a first use case). The inliner uses ephemeral value analysis to avoid counting instructions contributing to the invariants against the inlining score.
Details
Diff Detail
Event Timeline
docs/LangRef.html | ||
---|---|---|
9077 | as the -> at the | |
9077 | Is the return value is always equal to the first argument, regardless the offset? | |
9089–9092 | Minus? I think it should be 'plus'. We aren't passing the negation of the offset, are we? | |
lib/Analysis/BasicAliasAnalysis.cpp | ||
220 ↗ | (On Diff #384) | This function is duplicated in this patch. |
docs/LangRef.html | ||
---|---|---|
9083–9085 | What about adding explicitly: "If the condition does not hold during execution, the behavior is undefined"? | |
include/llvm/Analysis/EphemeralValues.h | ||
9–11 | Three slashes and '\file', please. | |
35 | Three slashes please. | |
44 | Does this need 'explicit'? | |
include/llvm/Analysis/InlineCost.h | ||
110 | Three slashes... | |
lib/Analysis/CaptureTracking.cpp | ||
139–142 ↗ | (On Diff #384) | Did you really intend to just move this piece of code a few lines down? |
lib/Analysis/EphemeralValues.cpp | ||
10–12 | Three slashes and '\file', please. | |
lib/Analysis/InlineCost.cpp | ||
47 | Three slashes, please. | |
lib/CodeGen/IntrinsicLowering.cpp | ||
458 | Please correct the comment (something like "strip out these intrinsics") | |
lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | ||
5023 | Please correct the comment. | |
lib/Transforms/Scalar/AlignmentInvProp.cpp | ||
10–12 | Three slashes and '\file' | |
243 | Is the '+' required here? |
Thanks!
Chandler was requested (on IRC) that I split this into 4 patches. I'll do that now, then I'll work on your comments.
-Hal
- Original Message -----
From: "Dmitri Gribenko" <gribozavr@gmail.com>
To: hfinkel@anl.gov
Cc: gribozavr@gmail.com, llvm-commits@cs.uiuc.edu, chandlerc@gmail.com
Sent: Wednesday, December 5, 2012 4:06:31 PM
Subject: Re: [PATCH] Invariants (and Assume Aligned) - LLVMComment 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?
Thanks!
Chandler was requested (on IRC) that I split this into 4 patches. I'll do that now, then I'll work on your comments.
-Hal
- Original Message -----
From: "Dmitri Gribenko" <gribozavr@gmail.com>
To: hfinkel@anl.gov
Cc: gribozavr@gmail.com, llvm-commits@cs.uiuc.edu, chandlerc@gmail.com
Sent: Wednesday, December 5, 2012 4:06:31 PM
Subject: Re: [PATCH] Invariants (and Assume Aligned) - LLVMComment 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?
FYI, the changes to LangRef need to be done against LangRef.rst now that LangRef.html is gone.
include/llvm/Analysis/EphemeralValues.h | ||
---|---|---|
46–51 | Three slashes and \ instead of @. |
as the -> at the