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

[David Li via llvm-commits]

davidxl 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?
----------------
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.


http://reviews.llvm.org/D19300