[PATCH] D105142: RFC: Implementing new mechanism for hard register operands to inline asm as a constraint.

Tue Jun 29 12:18:56 PDT 2021

anirudhp created this revision.
Herald added subscribers: pengfei, jfb, tpr.
anirudhp requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Relevant RFCs posted here

https://lists.llvm.org/pipermail/llvm-dev/2021-June/151370.html
https://gcc.gnu.org/pipermail/gcc/2021-June/236269.html

- This is put up as an RFC patch to get feedback about the introduction of a new inline asm constraint which supports hard register operands
- This is mostly a clang change, since the LLVM IR for the inline assembly already supports the {...} syntax which the backend recognizes. This change merely completes the loop in terms of introducing a user facing constraint which maps to it.

The following design decisions were taken for this patch:

For the Sema side:

- We validate the "{" constraint using a two-phase validation approach. Firstly, we check if there is a target dependent implementation to handle the {....} constraint. If it fails, then we move on to a target agnostic check where we parse and validate the {....} constraint.
- Why do we do this? Well, there are some targets which already process the {...} as a user facing constraint. For example the AMDGPU target. It supports syntax of the form {register-name} as well as {register-name[...]}. Moving this implementation to the target agnostic side seems to set a precedent, that we can keep extending the target agnostic implementation based on new cases for certain targets, which would be better served of moving it to the respective target.
- In terms of the target agnostic validation, we simply check for the following syntax {.*}. The parsed out content within the two curly braces is checked to see whether its a "valid GCC register".

For the Clang CodeGen side:

- Most of the work is done in the `AddVariableConstraints` function in CGStmt.cpp, which is responsible for emitting the LLVM IR corresponding to the usage of an actual register that the backend can use. Coincidentally, the LLVM IR is also {...}. As mentioned above, this is essentially mapping the LLVM Inline Assembly IR back to a user facing inline asm constraint.
- Within this function we add in logic to check if the constraint is of the form [&]{...} in addition to the "register asm" construct.
- A scenario where it was applicable to apply both the "register asm" construct and the "hard register inline asm constraint" was diagnosed as an unsupported error because there's no way the compiler will know which register the user meant. The safest option here is to error out explicitly, and put onus back on the user.
- To achieve this, and refactor it in a nice way, most of the logic pertaining to the "register asm" construct has been moved into a separate function called `ShouldApplyRegisterVariableConstraint` which deduces whether to apply the "register asm" construct or not.
- Furthermore, the GCCReg field is set with the "Register" only if the register is validated to be a "valid GCC Register" type. To me it doesn't make a lot of sense to set GCC Reg to something that might not necessarily be a "valid GCC register" as defined by respective targets. Would we have a case where we could have a {.*} constraint where the contents inside the curly brace is not a valid register? Yes. For example, The x86 target supports the "@cca" constraint, which is validated on the Sema side as a valid constraint, and before processing is converted to "{@cca}" (via the `convertAsmConstraint` function. "@cca" is not a valid "GCC register name". So in these case, we'll simply emit the constraint without setting GCCReg (with the assumption that the respective backends deal with it appropriately)

Tests:

- Various tests were updated to account for the new behaviour.
- I added a few SystemZ tests because we work on the Z backend, but I have no issues adding testing for multiple targets.
- There were a few mentions of "waiting" for the GCC implementation of the same to land before the Clang side lands. As mentioned above, the intent to implement it on the GCC side has already been put forward via the RFC. I have no issues "parking" this implementation until its ready to be merged in. However, it might be good to hash out any open questions/concerns in the interim.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105142

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/SystemZ/systemz-inline-asm-02.c
  clang/test/CodeGen/SystemZ/systemz-inline-asm.c
  clang/test/CodeGen/aarch64-inline-asm.c
  clang/test/CodeGen/asm-goto.c
  clang/test/CodeGen/ms-intrinsics.c
  clang/test/CodeGen/z-hard-register-inline-asm.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105142.355321.patch
Type: text/x-patch
Size: 20504 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210629/52c6a45e/attachment-0001.bin>