[PATCH] D17317: [attrs] Handle convergent CallSites.

[Chandler Carruth via llvm-commits]

chandlerc added inline comments.

================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:952-958
@@ +951,9 @@
+  // SCC.
+  auto IsConvergentNonSCCCall = [&](Instruction &I) {
+    CallSite CS(&I);
+    if (!CS || !CS.isConvergent())
+      return false;
+    Function *Callee = CS.getCalledFunction();
+    return !Callee || SCCNodes.count(Callee) == 0;
+  };
+
----------------
When these become multi-line lambdas with names and potential for confusing double negation, I have to say I find the early exit from a range based for loop substantially easier to read...

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:2039-2042
@@ +2038,6 @@
+    if (CS.isConvergent() && !CalleeF->isConvergent()) {
+      DEBUG(dbgs() << "Removing convergent attr from instr "
+                   << CS.getInstruction() << "\n");
+      CS.setNotConvergent();
+      Changed = true;
+    }
----------------
It's not clear that it is correct to fall through here... Usually these things go back onto a worklist and get visited again anyways. Is there a reason you chose this particular pattern?


http://reviews.llvm.org/D17317