Better global handling and fix caching bug #30
Conversation
| } | ||
| } | ||
|
|
||
| if (auto ce = dyn_cast<ConstantExpr>(val)) { |
There was a problem hiding this comment.
Comments needed, especially before conditionals that involve recursive calls.
| @@ -589,6 +628,8 @@ bool isconstantValueM(Value* val, SmallPtrSetImpl<Value*> &constants, SmallPtrSe | |||
| continue; | |||
| if (fnp->getIntrinsicID() == Intrinsic::memcpy && call->getArgOperand(0) != val && call->getArgOperand(1) != val) | |||
There was a problem hiding this comment.
For skipped intrinsics, a comment should be present describing the property the intrinsic has that lets us skip it. This lets people understand when they can add a "missing" intrinsic.
There was a problem hiding this comment.
Not skipping intrinsic, but rather special casing memcpy/memmove to say that the size variable is not made active even if other arguments are active
There was a problem hiding this comment.
My point was: if you are handling a bunch of things similarly (e.g. intrinsics) and its too cumbersome to give an explicit comment for each one, then an explanation for the group is good enough --- and should exist anyways so that folks understand what determines membership in the group of similar cases.
There was a problem hiding this comment.
Agreed re adding a comment, just wanted to clarify what code was doing
No description provided.