This is an archive of the discontinued LLVM Phabricator instance.

[MSAN] Handle x86 {round,min,max}sd intrinsics
ClosedPublic

Authored by guiand on Jun 23 2020, 11:08 AM.

Details

Summary

These need special handling over the simple vector intrinsics as they behave more like a shuffle operation: taking the top half of the vector from one input, and the bottom half separately. Previously, these were being handled as though all bits of all operands were combined.

Diff Detail

Event Timeline

guiand created this revision.Jun 23 2020, 11:08 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 23 2020, 11:08 AM
guiand updated this revision to Diff 272776.Jun 23 2020, 11:13 AM

Ran clang-format

The test should be in LLVM, under test/Instrumentation/MemorySanitizer

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
3069

You probably want to insert in First, not Second.

Is the generated code any better if you OR the vectors, and then shuffle to put the top element of First into the top element of the output? That's what LLVM generates if I express this logic in C.

guiand marked an inline comment as done.Jun 23 2020, 1:21 PM
guiand added inline comments.
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
3069

The codegen is basically identical either way, but if you'd like I can still upload a patch to change these into shufflevector instructions.

guiand updated this revision to Diff 272842.Jun 23 2020, 3:08 PM

Use shufflevector, move test over to IR

guiand marked an inline comment as done.Jun 23 2020, 3:08 PM
eugenis added inline comments.Jun 23 2020, 3:34 PM
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
3069

This is much better.
Use makeArrayRef({2, 1}).

3323

Unrelated change.

llvm/test/Instrumentation/MemorySanitizer/vector_sd.ll
17 ↗(On Diff #272842)

pass the vectors as arguments, it will make the test case a lot simpler

guiand updated this revision to Diff 272855.Jun 23 2020, 3:51 PM

Addressed comments

guiand marked 4 inline comments as done.Jun 23 2020, 3:52 PM
guiand added inline comments.
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
3323

Whoops!

eugenis accepted this revision.Jun 23 2020, 4:17 PM

LGTM

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
3069

llvm:: is unnecessary, and <int> is probably too

This revision is now accepted and ready to land.Jun 23 2020, 4:17 PM
This revision was automatically updated to reflect the committed changes.
guiand marked an inline comment as done.