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

[Philip Reames via llvm-commits]

On 04/21/2016 02:17 PM, Sanjay Patel via llvm-commits wrote:
> 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. :)
Please do.  After reading through the linked discussion, I'm not really 
following the motivation for removing it.  I don't have any particular 
objection, I just don't understand the reasoning.
>
> http://reviews.llvm.org/D19300
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits