This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Add support for the s,j,x,N,O inline asm constraints
ClosedPublic

Authored by dcandler on Aug 7 2019, 5:12 AM.

Details

Summary

A number of inline assembly constraints are currently supported by LLVM, but rejected as invalid by Clang:

Target independent constraints:

s: An integer constant, but allowing only relocatable values

ARM specific constraints:

j: An immediate integer between 0 and 65535 (valid for MOVW)
x: A 32, 64, or 128-bit floating-point/SIMD register: s0-s15, d0-d7, or q0-q3
N: An immediate integer between 0 and 31 (Thumb1 only)
O: An immediate integer which is a multiple of 4 between -508 and 508. (Thumb1 only)

This patch adds support to Clang for the missing constraints along with some checks to ensure that the constraints are used with the correct target and Thumb mode, and that immediates are within valid ranges (at least where possible). The constraints are already implemented in LLVM, but just a couple of minor corrections to checks (V8M Baseline includes MOVW so should work with 'j', 'N' and 'O' shouldn't be valid in Thumb2) so that Clang and LLVM are in line with each other and the documentation.

Diff Detail

Event Timeline

dcandler created this revision.Aug 7 2019, 5:12 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 7 2019, 5:12 AM
compnerd added inline comments.Aug 7 2019, 3:16 PM
clang/lib/Basic/Targets/ARM.cpp
904

I would hoist the comment above the if as that results in a more readable condition.

938

Can we leave this as a FIXME? This needs additional validation on the input.

941

Please hoist the comment above the if.

948

Similar

dcandler marked 5 inline comments as done.Aug 8 2019, 12:06 PM
dcandler added inline comments.
clang/lib/Basic/Targets/ARM.cpp
938

I think it's not just the M constraint that requires additional validation. Most of these immediate constraints require values that can fit in specific encodings to be valid, or have properties like being a multiple of a number, but setRequiresImmediate at present can only check against a min/max or exact values.

dcandler updated this revision to Diff 214205.Aug 8 2019, 12:06 PM
dcandler marked an inline comment as done.

Adjusted the formatting on some comment lines, and added FIXMEs for all the constraints that require additional validation to clarify what is still needed and where.

Ping. @compnerd any other changes before this could be accepted?

ostannard accepted this revision.Sep 3 2019, 5:20 AM

LGTM with one small change.

clang/test/Sema/arm_inline_asm_constraints.c
48

Copy-paste error, should be "between 0 and 255".

This revision is now accepted and ready to land.Sep 3 2019, 5:20 AM
This revision was automatically updated to reflect the committed changes.