[PATCH] D50342: Changed how LLVM IR was generated to increase vectorization

Emmett Neyman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 6 15:50:33 PDT 2018


emmettneyman added inline comments.


================
Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:127
+            << "define void @foo(i32* %a, i32* %b, i32* %c, i64 %s) {\n"
+            << "%1 = icmp sgt i64 %s, 0\n"
+            << "br i1 %1, label %start, label %end\n"
----------------
morehouse wrote:
> Should `%s` be signed?  Do we want unsigned compare here?
I don't think it matters since `%s` will always be positive (it will always be equal to 
`kArraySize` from `input_arrays.h`). 


================
Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:134
+            << "end:\n"
+            << "ret void\n"
             << "loop:\n"
----------------
morehouse wrote:
> Seems like the `endloop` label is unnecessary.  Does this help vectorize?  If not, lets get rid of unconditional jumps to the next line.
I initially had it this way since that's how `-emit-llvm` generated the code. But it doesn't seem like it's necessary for loops to be vectorized upon further inspection.


================
Comment at: clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp:138
             << x.statements()
-            << "%z = add i64 1, %ct\n"
-            << "store i64 %z, i64* %i\n"
-            << "br label %loop\n\n"
-            << "endloop:\n"
-            << "ret void\n}\n";
+            << "%ctnew = add nuw nsw i64 %ct, 1\n"
+            << "%j = icmp eq i64 %ctnew, %s\n"
----------------
morehouse wrote:
> This will make overflow undefined... Isn't that the opposite of what we want?  That will permit LLVM to assume overflow never happens and modify the code in new ways based on that assumption.
That's a good point. It probably won't matter since this addition will never overflow (we're just starting at zero and incrementing by one). This is what was generated by `-emit-llvm`. I'll remove it to reduce confusion.


Repository:
  rC Clang

https://reviews.llvm.org/D50342





More information about the cfe-commits mailing list