This is an archive of the discontinued LLVM Phabricator instance.

[X86][AVX512] Removing llvm x86 intrinsics for _mm_mask_move_{ss|sd} intrinsics.
ClosedPublic

Authored by aymanmus on Oct 30 2016, 5:16 AM.

Diff Detail

Event Timeline

aymanmus updated this revision to Diff 76333.Oct 30 2016, 5:16 AM
aymanmus retitled this revision from to [X86][avx512] Removing llvm x86 intrinsics for _mm_mask_move_{ss|sd} intrinsics..
aymanmus updated this object.
aymanmus added reviewers: zvi, igorb, m_zuckerman, delena.
aymanmus added a subscriber: llvm-commits.
delena added inline comments.Oct 30 2016, 5:27 AM
lib/IR/AutoUpgrade.cpp
680

All variables should start from a capital letter.

aymanmus updated this revision to Diff 76354.EditedOct 31 2016, 12:57 AM
aymanmus retitled this revision from [X86][avx512] Removing llvm x86 intrinsics for _mm_mask_move_{ss|sd} intrinsics. to [X86][AVX512] Removing llvm x86 intrinsics for _mm_mask_move_{ss|sd} intrinsics..

Variables to start with capital letter.

aymanmus marked an inline comment as done.Oct 31 2016, 1:02 AM
igorb accepted this revision.Nov 6 2016, 10:53 PM
igorb edited edge metadata.

LGTM

This revision is now accepted and ready to land.Nov 6 2016, 10:53 PM

Two minor comments. But LGTM otherwise.

lib/IR/AutoUpgrade.cpp
341

This is the whole name of the intrinsic so startswith is unnecessary. You should be able to combine both checks to a single Name.startswith("avx512.mask.move.s").

aymanmus updated this revision to Diff 77160.Nov 8 2016, 1:01 AM
aymanmus edited edge metadata.
aymanmus marked an inline comment as done.
This revision was automatically updated to reflect the committed changes.

This change has been reverted due to failure in build.
compilers do not accept nesting levels greater that 127, and the file AutoUpgrade.cpp contains a long sequence of "if else" statements which the compiler considers too deep.

The error I got:
"FAILED: lib/IR/CMakeFiles/LLVMCore.dir/AutoUpgrade.cpp.obj
C:\Buildbot\Slave\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\lib\IR\AutoUpgrade.cpp(1529): fatal error C1061: compiler limit: blocks nested too deeply"

A heavy refactor to this piece of code should be considered as an effort to reduce the use of llvm x86 intrinsics is being initiated, and as a result many new cases will be added to the same control flow in the future.

After commit that reduced the number of "if else" statements (r286768) this patch was recommitted as r287087.