This is an archive of the discontinued LLVM Phabricator instance.

[AVX-512] Fix the operand order for all calls to __builtin_ia32_vfmaddss3_mask.
ClosedPublic

Authored by craig.topper on Oct 23 2016, 11:12 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper retitled this revision from to [AVX-512] Fix the operand order for all calls to __builtin_ia32_vfmaddss3_mask..
craig.topper updated this object.
craig.topper added reviewers: delena, igorb.
craig.topper added a subscriber: cfe-commits.
delena added inline comments.Oct 24 2016, 11:28 AM
lib/Headers/avx512fintrin.h
8394 ↗(On Diff #75557)

Using vfmaddss3_mask is wrong in this case.
It will propagate -B to lower bits if mask==0.

8453 ↗(On Diff #75557)

This one is, probably, also wrong, because we should copy the upper bits of __A as is.

IF k[0]
dst[31:0] := -(a[31:0] * b[31:0]) + c[31:0]
ELSE
dst[31:0] := c[31:0]
FI
dst[127:32] := a[127:32]
dst[MAX:128] := 0

craig.topper added inline comments.Oct 24 2016, 12:20 PM
lib/Headers/avx512fintrin.h
8394 ↗(On Diff #75557)

I dont' understand this. The passthru should be from the first argument. This call should do

if (mask)

result = _W * _A - _B

else

result = _W;
8453 ↗(On Diff #75557)

Agree this is also wrong. I was going to try to fix one intrinsic at a time. We have some problems with __builtin_ia32_vfmaddss3_mask3 usages as well.

delena accepted this revision.Oct 25 2016, 3:49 AM
delena edited edge metadata.

LGTM. Agree, one-by-one.

This revision is now accepted and ready to land.Oct 25 2016, 3:49 AM
This revision was automatically updated to reflect the committed changes.