[PATCH] D33737: [InstSimplify] Don't constant fold or DCE calls that are marked nobuiltin

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor added a comment.

In https://reviews.llvm.org/D33737#769328, @efriedma wrote:

> The other alternative is that you could pass in an AttributeList (the result of getAttributes()) to the functions... that makes it more clear what exactly is getting used, but it's more complicated (and probably involves a bunch of refactoring to expose a function to check isNoBuiltin given a Function/AttributeList pair).
>
> But either way, I would prefer a solution that involves checking for isNoBuiltin() in as few places as possible.


I'd like to avoid the AttributeList because of the complications when the function declaration is nobuiltin but the CallSite is builtin.  I'll implement your first suggestion and post an updated patch.  I agree that it is better to have fewer places to check the attributes.



================
Comment at: lib/Transforms/Utils/Evaluator.cpp:363
+      // Cannot evaluate nobuiltin functions.
+      if (CS.isNoBuiltin()) {
+        DEBUG(dbgs() << "Can not evaluate nobuiltin call\n");
----------------
efriedma wrote:
> This isn't doing what you want: we don't call ConstantFoldCall on all paths.
My reasoning is that if the call is marked as nobuiltin then we can't make any assumptions about what it does.  To be honest though, I wasn't sure about this function.

Is this used by LLDB and similar clients for live code evaluation?


Repository:
  rL LLVM

https://reviews.llvm.org/D33737