[PATCH] D19299: lower __builtin_expect() directly to prof metadata instead of LLVM intrinsic

[Sanjay Patel via cfe-commits]

spatel added inline comments.

================
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
----------------
davidxl wrote:
> 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.
Ah, I hadn't considered extending builtin_expect in that way. In that case, it does make sense to leave it as-is here and use prof metadata because it's already set up for a list of values.

I'll clean up and re-post the patch. Thanks everyone for the reviews!


http://reviews.llvm.org/D19299