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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
