[PATCH] D21110: [CFLAA] Add yet another StratifiedAttr

Jia Chen via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 9 16:21:12 PDT 2016


grievejia added inline comments.

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1024
@@ +1023,3 @@
+      // TODO: do we need to filter out non-pointer values here?
+      Builder.addAttributesBelow(&Val, AttrUnknown);
+    }
----------------
george.burgess.iv wrote:
> Insertion shouldn't be a problem, since the only things we insert after the declaration of this are isolated nodes. Also, I think merging should be fine, since we never actually merge attributes, but I could be wrong.
> 
> Either way, I'm happy to leave this as-is and revisit it if it becomes a problem (since the worst case seems to be that we end up with a handful of "useless" bytes in a StratifiedSets instance)
OK I see your point (how many times did I say this... I should really work on my comprehension skills). 

The thing is, addAttributeBelow() is written with the assumption that useful nodes will be inserted below some time in the future. I guess you are right that right now this assumption does not hold. However, I'm going to mark some actual arguments of CallInst AttrEscape in the next patch, and depending on the way this is implemented we may end up calling addBelow() after addAttributeBelow() at some point. 


http://reviews.llvm.org/D21110





More information about the llvm-commits mailing list