[ARM] Skip inline asm memory operands in DAGToDAGISel

[rovka]

Summary

The current logic for handling inline asm operands in DAGToDAGISel interprets
the operands by looking for constants, which should represent the flags
describing the kind of operand we're dealing with (immediate, memory, register
def etc). The operands representing actual data are skipped only if they are
non-const, with the exception of immediate operands which are skipped explicitly
when a flag describing an immediate is found.

The oversight is that memory operands may be const too (e.g. for device drivers
reading a fixed address), so we should explicitly skip the operand following a
flag describing a memory operand. If we don't, we risk interpreting that
constant as a flag, which is definitely not intended.

[rengolin]

Otherwise, LGTM. Thanks!

test/CodeGen/ARM/inlineasm3.ll

128

Can you add the CHECK lines that explicitly say what you expect to see?

[vitalybuka]

Reverted r275776 and r275777

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/12458

FAIL: LLVM :: CodeGen/ARM/inlineasm3.ll (4451 of 17333)
******************** TEST 'LLVM :: CodeGen/ARM/inlineasm3.ll' FAILED ********************
Script:
--
/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build_msan/./bin/llc -mtriple=arm-eabi -float-abi=soft -mattr=+neon,+v6t2 -no-integrated-as /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/test/CodeGen/ARM/inlineasm3.ll -o -   | /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm_build_msan/./bin/FileCheck /mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/test/CodeGen/ARM/inlineasm3.ll
--
Exit Code: 1

Command Output (stderr):
--
/mnt/b/sanitizer-buildbot2/sanitizer-x86_64-linux-bootstrap/build/llvm/test/CodeGen/ARM/inlineasm3.ll:10:10: error: expected string not found in input
; CHECK: vmov.I64 q15, #0
         ^
<stdin>:1:4: note: scanning from here
 .text
   ^
<stdin>:8:8: note: possible intended match here
 .eabi_attribute 17, 1 @ Tag_ABI_PCS_GOT_use

[vitalybuka]

This comment was removed by vitalybuka.

[rengolin]

As Paul identified, the like:

CHECK-LABEL: t

could be detecting the wrong thing and the next CHECK: vmov failed.

Diana, this doesn't seem to be a problem with the patch itself, but with the second patch to insert the additional CHECK lines.

I won't reapply either of the patches, and will let you sort out the problem tomorrow.

cheers,
--renato

lib/Target/ARM/ARMISelDAGToDAG.cpp

 
     unsigned DefIdx = 0;
     bool IsTiedToChangedOp = false;
     // If it's a use that is tied with a previous def, it has no
     // reg class constraint.
     if (Changed && InlineAsm::isUseOperandTiedToDef(Flag, DefIdx))
       IsTiedToChangedOp = OpChanged[DefIdx];
 
+    // Memory operands to inline asm in the SelectionDAG are modeled with two
+    // operands: a constant of value InlineAsm::Kind_Mem followed by the input
+    // operand. If we get here and we have a Kind_Mem, skip the next operand (so
+    // it doesn't get misinterpreted), and continue. We do this here because
+    // it's important to update the OpChanged array correctly before moving on.
+    if (Kind == InlineAsm::Kind_Mem) {
+      SDValue op = N->getOperand(++i);
+      AsmNodeOperands.push_back(op);
+      continue;
+    }
+
     if (Kind != InlineAsm::Kind_RegUse && Kind != InlineAsm::Kind_RegDef
         && Kind != InlineAsm::Kind_RegDefEarlyClobber)
       continue;
 
     unsigned RC;
     bool HasRC = InlineAsm::hasRegClassConstraint(Flag, RC);
     if ((!IsTiedToChangedOp && (!HasRC || RC != ARM::GPRRegClassID))
         || NumRegs != 2)

test/CodeGen/ARM/inlineasm3.ll

 
 define <4 x i32> @t11(i32* %p) nounwind {
 entry:
 ; CHECK: t11
 ; CHECK: vld1.s32 {d16[], d17[]}, [r0]
   %0 = tail call <4 x i32> asm "vld1.s32 {${0:e}[], ${0:f}[]}, [$1]", "=w,r"(i32* %p) nounwind
   ret <4 x i32> %0
 }
+
+; Bugzilla PR26038
+
+define i32 @fn1() local_unnamed_addr nounwind {
+entry:
+  %0 = tail call i32 asm "ldrh  $0, $1", "=r,*Q"(i8* inttoptr (i32 5 to i8*)) nounwind
+  ret i32 %0
+}