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

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

Hey, I can almost sign off on this, but two of the comments are not clear.  I'm happy to provide suggestions if you like.



================
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(
----------------
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.


================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:1139
+        // Remove attribute from inference on other functions (invalidate)
+        // since we have attribute assumptions already violated.
+        llvm::erase_if(InferInSCC, [&ID](const InferenceDescriptor &D) {
----------------
This comment needs to be reworded, sorry.


Repository:
  rL LLVM

https://reviews.llvm.org/D44415