[llvm] r204861 - Fix a problem with the ARM assembler incorrectly matching a

[Kevin Enderby]

Author: enderby
Date: Wed Mar 26 16:54:11 2014
New Revision: 204861

URL: http://llvm.org/viewvc/llvm-project?rev=204861&view=rev
Log:
Fix a problem with the ARM assembler incorrectly matching a
vector list parameter that is using all lanes "{d0[], d2[]}" but can
match and instruction with a ”{d0, d2}" parameter.

I’m finishing up a fix for proper checking of the unsupported
alignments on vld/vst instructions and ran into this.  Thus I don’t
have a test case at this time.  And adding all code that will
demonstrate the bug would obscure the very simple one line fix.
So if you would indulge me on not having a test case at this
time I’ll instead offer up a detailed explanation of what is
going on in this commit message.

This instruction:

	vld2.8  {d0[], d2[]}, [r4:64]

is not legal as the alignment can only be 16 when the size is 8.
Per this documentation:

A8.8.325 VLD2 (single 2-element structure to all lanes)
 <align> The alignment. It can be one of:
16 2-byte alignment, available only if <size> is 8, encoded as a = 1.
32 4-byte alignment, available only if <size> is 16, encoded as a = 1.
64 8-byte alignment, available only if <size> is 32, encoded as a = 1.
omitted Standard alignment, see Unaligned data access on page A3-108.

So when code is added to the llvm integrated assembler to not match
that instruction because of the alignment it then goes on to try to match
other instructions and comes across this:

	vld2.8  {d0, d2}, [r4:64]

and and matches it. This is because of the method
ARMOperand::isVecListDPairSpaced() is missing the check of the Kind.
In this case the Kind is k_VectorListAllLanes . While the name of the method
may suggest that this is OK it really should check that the Kind is
k_VectorList.

As the method ARMOperand::isDoubleSpacedVectorAllLanes() is what was
used to match {d0[], d2[]}  and correctly checks the Kind:

  bool isDoubleSpacedVectorAllLanes() const {
    return Kind == k_VectorListAllLanes && VectorList.isDoubleSpaced;
  }

where the original ARMOperand::isVecListDPairSpaced() does not check
the Kind:

  bool isVecListDPairSpaced() const {
    if (isSingleSpacedVectorList()) return false;
    return (ARMMCRegisterClasses[ARM::DPairSpcRegClassID]
              .contains(VectorList.RegNum));
  }

Jim Grosbach has reviewed the change and said:  Yep, that sounds right. …
And by "right" I mean, "wow, that's a nasty latent bug I'm really, really
glad to see fixed." :)

rdar://16436683

Modified:
    llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp

Modified: llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp?rev=204861&r1=204860&r2=204861&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp (original)
+++ llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp Wed Mar 26 16:54:11 2014
@@ -1366,6 +1366,7 @@ public:
   }
 
   bool isVecListDPairSpaced() const {
+    if (Kind != k_VectorList) return false;
     if (isSingleSpacedVectorList()) return false;
     return (ARMMCRegisterClasses[ARM::DPairSpcRegClassID]
               .contains(VectorList.RegNum));