This is an archive of the discontinued LLVM Phabricator instance.

Invariants (and Assume Aligned) - LLVM
AbandonedPublic

Authored by hfinkel on Nov 30 2012, 3:58 PM.

Details

Reviewers
silvas
Summary

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.

Diff Detail

Event Timeline

gribozavr added inline comments.Dec 1 2012, 4:35 AM
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.

hfinkel updated this revision to Unknown Object (????).Dec 5 2012, 12:43 PM

Changed to using @llvm.invariant; added analysis for ephemeral values.

gribozavr added inline comments.Dec 5 2012, 2:06 PM
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) - LLVM

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

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) - LLVM

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

silvas requested changes to this revision.Dec 7 2012, 10:53 AM

FYI, the changes to LangRef need to be done against LangRef.rst now that LangRef.html is gone.

Bigcheese added inline comments.Dec 7 2012, 1:18 PM
include/llvm/Analysis/EphemeralValues.h
46–51

Three slashes and \ instead of @.

hfinkel abandoned this revision.Jul 12 2014, 8:58 AM

(this patchset has been split, and will be updated; abandoning...)