Page MenuHomePhabricator

[AsmParser][AVX512]Enhance OpMask/Zero/Merge syntax check rubostness
ClosedPublic

Authored by coby on Jul 24 2017, 12:16 AM.

Details

Summary

Adopt a more strict approach regarding what marks should/can appear after a destination register, when operating upon an AVX512 platform.

Diff Detail

Repository
rL LLVM

Event Timeline

coby created this revision.Jul 24 2017, 12:16 AM
coby added a reviewer: igorb.Jul 29 2017, 12:11 PM
m_zuckerman added inline comments.Jul 31 2017, 12:35 PM
test/MC/X86/avx512-err.s
10 ↗(On Diff #107852)

why is this error a true?
according to the ISA, the instruction can optionally have op mask.

m_zuckerman edited edge metadata.EditedJul 31 2017, 12:38 PM

Expect (see above ) from that Look good to me

coby added inline comments.Jul 31 2017, 1:17 PM
test/MC/X86/avx512-err.s
10 ↗(On Diff #107852)

The error due to the misplacing of the rounding (rn-sae) mark (line 11).
Only an OpMask / Zero mark is allowed to immediately follow a destination operand.

m_zuckerman added inline comments.Jul 31 2017, 10:33 PM
test/MC/X86/avx512-err.s
8 ↗(On Diff #107852)

So why this is an error? you mustn't have the {z} mask before the {rn-sae}.
Again according to the ISA section 2.5.4
https://software.intel.com/sites/default/files/managed/07/b7/319433-023.pdf
for example:
vaddps zmm7 {k6}, zmm2, zmm4, {rd-sae}

10 ↗(On Diff #107852)

can you add also the intel syntax test since it is written in a different way? The location of the mask is the first and not the last.

For example:
vaddps zmm7 {k6}, zmm2, zmm4, {rd-sae}

and see that you are getting the same error when you insert
vaddps zmm7 , zmm2, zmm4, {rd-sae}

m_zuckerman added inline comments.Jul 31 2017, 10:35 PM
test/MC/X86/avx512-err.s
8 ↗(On Diff #107852)

you don't must the {z} before the {rn-sae}

coby added inline comments.Aug 1 2017, 12:00 AM
test/MC/X86/avx512-err.s
8 ↗(On Diff #107852)

Examine the example you took out of the Spec:
"vaddps zmm7 {k6}, zmm2, zmm4, {rd-sae}" (Intel style)
The rounding mark is stated as a distinctive operand of the instruction (note the comma).
As said above - only OpMask/Zero marks can immediately follow a destination
immediately ::= As a mark stating properties of the followed operand (or, if you want, not as a distinctive operand)

10 ↗(On Diff #107852)

The code flow dictates that this added check will occur after any operand of an AVX512 instruction (more precisely - if this operand is followed by an alleged curly braces marks), so it catches both mischief conducted on Intel and GNU. In other words - it matters not whether the faulty occurs on the last/first operand

coby updated this revision to Diff 110169.Aug 8 2017, 3:49 AM

Added intel-dialect test as well

m_zuckerman accepted this revision.Aug 8 2017, 5:44 AM
This revision is now accepted and ready to land.Aug 8 2017, 5:44 AM
This revision was automatically updated to reflect the committed changes.