davidxl added inline comments. ================ Comment at: lib/CodeGen/CGBuiltin.cpp:636 @@ -640,1 +635,3 @@ + case Builtin::BI__builtin_unpredictable: case Builtin::BI__builtin_expect: { + // Always return the first argument. LLVM does not handle these builtins. ---------------- Can this be reordered with unpredicatle case so that it can handle arg 1 and fall through?
================ Comment at: lib/CodeGen/CGStmt.cpp:1550 @@ -1549,3 +1549,3 @@ // If the switch has a condition wrapped by __builtin_unpredictable, // create metadata that specifies that the switch is unpredictable. ---------------- update the comment here. ================ Comment at: lib/CodeGen/CGStmt.cpp:1562 @@ +1561,3 @@ + + // FIXME: builtin_expect should use the same metadata type as + // builtin_unpredictable and be handled above. For now, we're mimicking ---------------- I am not sure about this. builtin_expect can be used to do general value profiling annotation (single value). For instance, if (builtin_expect(a, 20) > 10) { } should have same effect as if (builtin_expect(a > 10), true) { } The above can be handled by gcc, but not LLVM. It can be useful for switch case annotation: switch (__builtin_expect(v, 20) ) { case 10: ... case 20: ... ... } where compiler can do switch peeling. Longer term, I am thinking extending builtin_expect to take list of values with probabilities and predicatibiity hint. ================ Comment at: lib/CodeGen/CGStmt.cpp:1566 @@ +1565,3 @@ + + // HACK: Hardcode the taken/not-taken weights based on the existing LLVM + // default values. This code is expected to be very temporary. Once we ---------------- Unpredicable meta data is probably not suitable for switch annotation. builtin_expect can be used to specify one case that is more likely to be taken thus helping switch lowering decision (not used in the cases such as if conversion). ================ Comment at: lib/CodeGen/CodeGenFunction.cpp:1312 @@ -1311,3 +1311,3 @@ // If the branch has a condition wrapped by __builtin_unpredictable, // create metadata that specifies that the branch is unpredictable. ---------------- Update comment here. ================ Comment at: lib/CodeGen/CodeGenFunction.cpp:1325 @@ +1324,3 @@ + + // FIXME: builtin_expect should use the same metadata type as + // builtin_unpredictable and be handled above. For now, we're mimicking ---------------- I suggest removing these comments. http://reviews.llvm.org/D19299 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits