This is an archive of the discontinued LLVM Phabricator instance.

Changed how LLVM IR was generated to increase vectorization
ClosedPublic

Authored by emmettneyman on Aug 6 2018, 10:53 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

emmettneyman created this revision.Aug 6 2018, 10:53 AM
morehouse added inline comments.Aug 6 2018, 11:19 AM
clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp
125 ↗(On Diff #159338)

What does pc mean in this triple? I'm used to seeing x86_64-unknown-linux-gnu.

126 ↗(On Diff #159338)

Does removing the variable names really make this easier to vectorize?

129 ↗(On Diff #159338)

Does removing branch names really make this easier to vectorize?

emmettneyman added inline comments.Aug 6 2018, 1:05 PM
clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp
125 ↗(On Diff #159338)

It's probably generating pc since I'm using my desktop. I'll change it to unknown so it's not platform-specific.

126 ↗(On Diff #159338)

It doesn't, I just thought it was cleaner and produced slightly smaller IR. It also more closely mimics the behavior of the -emit-llvm flag.

129 ↗(On Diff #159338)

Same answer as above. Should I change these back?

Changed pc to unknown

morehouse added inline comments.Aug 6 2018, 1:24 PM
clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp
129 ↗(On Diff #159338)

I think the previous labels were more human-readable.

Added back more descriptive variable and loop names

morehouse added inline comments.Aug 6 2018, 3:29 PM
clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp
127 ↗(On Diff #159389)

Should %s be signed? Do we want unsigned compare here?

134 ↗(On Diff #159389)

Seems like the endloop label is unnecessary. Does this help vectorize? If not, lets get rid of unconditional jumps to the next line.

138 ↗(On Diff #159389)

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.

emmettneyman added inline comments.Aug 6 2018, 3:50 PM
clang/tools/clang-fuzzer/proto-to-llvm/loop_proto_to_llvm.cpp
127 ↗(On Diff #159389)

I don't think it matters since %s will always be positive (it will always be equal to
kArraySize from input_arrays.h).

134 ↗(On Diff #159389)

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.

138 ↗(On Diff #159389)

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.

Some small fixes to improve simplicity of generated IR

morehouse accepted this revision.Aug 6 2018, 3:57 PM
This revision is now accepted and ready to land.Aug 6 2018, 3:57 PM
Rebased and ready to land
This revision was automatically updated to reflect the committed changes.