[PATCH] D50670: Implementation of nested loops in cxx_loop_proto

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.


================
Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:127
   }
+  inner_loop = true;
   return os;
----------------
Maybe this fixes the bug, but modifying `inner_loop` from different functions is still error-prone.

Please either make this a scoped variable (with a wrapper class that sets it to true in the constructor and sets it to false in the destructor), or make it a parameter.


================
Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:140
+     << "br label %inner_loop\n"
+     << "end:\n"
+     << "ret void\n"
----------------
I don't see any jumps to `end`.  I think this will be an infinite loop.


================
Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:143
+     << "outer_loop:\n"
+     << x.outer_statements()
+     << "%o_ct_new = add i64 %outer_ct, 1\n"
----------------
IIUC this creates loop structure always like this:

```
for (int i = 0; i < s; i++) {
  for (int j = 0; j < s; j++) {
    // statements
  }
  // statements
}
```

Maybe not necessary for this patch, but I'm curious if adding statements before the inner loop would exercise different coverage in the vectorizer.


================
Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:143
+     << "outer_loop:\n"
+     << x.outer_statements()
+     << "%o_ct_new = add i64 %outer_ct, 1\n"
----------------
morehouse wrote:
> IIUC this creates loop structure always like this:
> 
> ```
> for (int i = 0; i < s; i++) {
>   for (int j = 0; j < s; j++) {
>     // statements
>   }
>   // statements
> }
> ```
> 
> Maybe not necessary for this patch, but I'm curious if adding statements before the inner loop would exercise different coverage in the vectorizer.
Will all loops be double-nested now?


Repository:
  rC Clang

https://reviews.llvm.org/D50670