[PATCH] LLVM intrinsic for invariants

[hfinkel at anl.gov]

================
Comment at: docs/LangRef.rst:9383
@@ +9382,3 @@
+mathematical invariants that the optimizer can otherwise deduce or facts that
+are of little use to the optimizer.
+
----------------
Philip Reames wrote:
> Why?  Suggestion:
> Please note that ''llvm.assume'' is not free and may impact the effectiveness of the optimizer in hard to predict ways.  ''llvm.assume'' should be used to represent high level facts which are not otherwise available to the optimizer and only after careful consideration of the benefit and costs.  In particular, 'llvm.assume'' should not be used to...
> 
> 
> I'm not sure I really like my wording around 'free' above, but this is a starting point.  
Good idea. Instead of "free", I'll say something like, "The optimizer might limit the transformations performed on values used by the intrinsic in order to preserve the instructions only used to form the intrinsic's input argument, and this might prove undesirable if the extra information provided by that intrinsic does cause sufficient overall improvement."

================
Comment at: include/llvm/IR/Intrinsics.td:282
@@ +281,3 @@
+// control dependencies will be maintained.
+def int_assume        : Intrinsic<[], [llvm_i1_ty], []>;
+
----------------
Philip Reames wrote:
> I understand why you're doing this, but it honestly feels like a bit of a hack.  Is it possibly time to introduce a notion of control dependence that is not driven by memory access?  This is essentially what you've written, but there might be a better way to factor it.  
> 
> This doesn't need to be changed before submission, I'm just raising it for consideration and discussion.  
Fixing this is another major infrastructure project. :(

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:799
@@ -801,1 +798,3 @@
 
+static bool isAssumeIntrinsic(ImmutableCallSite CS) {
+  const IntrinsicInst *II = dyn_cast<IntrinsicInst>(CS.getInstruction());
----------------
Philip Reames wrote:
> I've noticed we have multiple isXIntrinsic functions scatter around the code base.  We might want to think about - in a separate change - creating an isIntrinsic(IntrinsicID) function on CallSite.  
Good idea; I'll queue that for later refactoring.

================
Comment at: lib/Transforms/Utils/Local.cpp:1199
@@ +1198,3 @@
+          if (isa<UndefValue>(II->getArgOperand(0)))
+            MakeUnreachable = true;
+          else if (ConstantInt *Cond =
----------------
Philip Reames wrote:
> Adding a comment on this case would be good.  Particularly given that it required discussion to arrive at this being the right semantic.  
Will do.

================
Comment at: test/CodeGen/Generic/assume.ll:1
@@ +1,2 @@
+; RUN: llc < %s
+
----------------
Philip Reames wrote:
> This test could be stronger.  Could you add a FileCheck here? 
No, this is a "generic lowering" test. It runs on all targets to make sure that it does not crash (that the code generator does *something*). You'll find many such tests in that directory.

http://reviews.llvm.org/D178