[PATCH] D19300: delete the llvm.expect intrinsic and its lowering pass

[Sanjay Patel via llvm-commits]

spatel added inline comments.

================
Comment at: lib/IR/AutoUpgrade.cpp:664
@@ +663,3 @@
+      // The expect intrinsic always lowered to its first argument.
+      // TODO: We could create the appropriate metadata here to better support
+      // legacy code?
----------------
davidxl wrote:
> spatel wrote:
> > davidxl wrote:
> > > Is there a reason not to upgrade it properly with meta data?
> > > 
> > > Another question for deprecation, can you just remove the documentation and leave the support for the intrinsic support for now?
> > AFAIK, we don't make any guarantees to support legacy IR, except that we don't die processing it or miscompile it, so this is the minimal patch. Given that we're still losing metadata and/or not using it in a lot of cases (PR27344), I figured we should just focus on improving that instead of spending time on backwards support. If others disagree, I can add it.
> > 
> > If we just remove the docs, we'd still be doing unnecessary work for every compile, right? I don't see any reason to take that path.
> > 
> > 
> Personally, I don't have issues with completely removing it -- but I can not speak for others in this aspect. IMO, removing documentation can be a safe first step -- the compile time impact should be minimal.
Let me send a note to the dev list about this patch. If anyone objects or would like us to implement this in stages, we should hear about it. :)


http://reviews.llvm.org/D19300