[PATCH] D44415: [PM][FunctionAttrs] add NoUnwind attribute inference to PostOrderFunctionAttrs pass

[Justin Lebar via Phabricator via llvm-commits]

jlebar accepted this revision.
jlebar added a comment.
This revision is now accepted and ready to land.

\o/



================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:1128
+    // All the attributes currently available for inference are good to go
+    // with this function unless explicitly skipped.
+    llvm::copy_if(
----------------
fedor.sergeev wrote:
> jlebar wrote:
> > jlebar wrote:
> > > Suggest moving the comment up one line and saying something like
> > > 
> > > ```For each attribute still in InferInSCC that doesn't skip F, check that the instructions in F are valid.```
> > Sorry, I think the reworded comment is still unclear.
> > 
> > If you're willing to tell me what you don't like about the suggestion I made earlier, perhaps we can come up with something we both like.
> Mostly I didnt like "instructions are valid" part, which is not what we are checking for.
> If you dont like my last variant - please, suggest your own that talks about attribute, not instruction.
What you came up with looks great to me.

Thank you for being patient with me -- I know I'm not an easy reviewer.


================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:1138
+        // Remove attribute from further inference on any other functions
+        // because attribute assumptions have just been violated.
+        llvm::erase_if(InferInSCC, [&ID](const InferenceDescriptor &D) {
----------------
lgtm, thanks again for being patient with me.


Repository:
  rL LLVM

https://reviews.llvm.org/D44415