Page MenuHomePhabricator

[ARM] Skip inline asm memory operands in DAGToDAGISel
ClosedPublic

Authored by rovka on Jul 7 2016, 11:44 AM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

rovka updated this revision to Diff 63102.Jul 7 2016, 11:44 AM
rovka retitled this revision from to [ARM] Skip inline asm memory operands in DAGToDAGISel.
rovka updated this object.
rovka added reviewers: jmolloy, bogner, echristo.
rovka added subscribers: llvm-commits, rengolin.

Otherwise, LGTM. Thanks!

test/CodeGen/ARM/inlineasm3.ll
128 ↗(On Diff #63102)

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

This revision was automatically updated to reflect the committed changes.
vitalybuka reopened this revision.EditedJul 18 2016, 12:52 PM
vitalybuka added a subscriber: 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
This comment was removed by vitalybuka.

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

This revision was automatically updated to reflect the committed changes.