Skip to content

[PPC][InlineASM] Mark the 'a' constraint as unsupported #96109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 24, 2024

Conversation

kamaub
Copy link
Contributor

@kamaub kamaub commented Jun 19, 2024

'a' is an input/ouput constraint for restraining assembly variables
to an indexed or indirect address operand. It previously was marked
as supported but would throw an assertion for unknown constraint type
in the back-end when this test case was compiled. This change marks it
as unsupported until we can add full support for address operands
constraining to the compiler code generation.

'a' is an input/ouput constraint for restraining assembly variables
to an indexed or indirect address operand. It previously was marked
as supported but would throw an assertion for unknown constraint type
in the back-end when this test case was compiled. This change marks it
as unsupported until we can add full support for address operands
constraining to the compiler code generation.
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:PowerPC clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 19, 2024

@llvm/pr-subscribers-backend-powerpc

@llvm/pr-subscribers-clang

Author: Kamau Bridgeman (kamaub)

Changes

'a' is an input/ouput constraint for restraining assembly variables
to an indexed or indirect address operand. It previously was marked
as supported but would throw an assertion for unknown constraint type
in the back-end when this test case was compiled. This change marks it
as unsupported until we can add full support for address operands
constraining to the compiler code generation.


Full diff: https://github.com/llvm/llvm-project/pull/96109.diff

2 Files Affected:

  • (modified) clang/lib/Basic/Targets/PPC.h (+3-1)
  • (added) clang/test/CodeGen/PowerPC/inline-asm-unsupported-constraint-error.c (+8)
diff --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index fc23c30c68523..e4d6a02386da5 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -305,9 +305,11 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public TargetInfo {
               // asm statements)
       Info.setAllowsMemory();
       break;
-    case 'R': // AIX TOC entry
     case 'a': // Address operand that is an indexed or indirect from a
               // register (`p' is preferable for asm statements)
+              // TODO: Add full support for this constraint
+      return false;
+    case 'R': // AIX TOC entry
     case 'S': // Constant suitable as a 64-bit mask operand
     case 'T': // Constant suitable as a 32-bit mask operand
     case 'U': // System V Release 4 small data area reference
diff --git a/clang/test/CodeGen/PowerPC/inline-asm-unsupported-constraint-error.c b/clang/test/CodeGen/PowerPC/inline-asm-unsupported-constraint-error.c
new file mode 100644
index 0000000000000..457908f016b0c
--- /dev/null
+++ b/clang/test/CodeGen/PowerPC/inline-asm-unsupported-constraint-error.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64le-unknown-unknown -verify %s
+// This test case exist to test marking the 'a' inline assembly constraint as
+// unsupported because powerpc previously marked it as supported.
+int foo(int arg){
+  asm goto ("bc %0,%1,%l[TEST_LABEL]" : : "a"(&&TEST_LABEL) : : TEST_LABEL); //expected-error {{invalid input constraint 'a' in asm}}
+  return 0;
+TEST_LABEL: return arg + 1;
+}

Copy link
Collaborator

@chenzheng1030 chenzheng1030 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except a nit.

@@ -0,0 +1,8 @@
// RUN: %clang_cc1 -emit-llvm -triple powerpc64le-unknown-unknown -verify %s
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you also add run lines for AIX 32 and 64 bit?

Adding test case run lines for aix 32 and 64 bit targets.
@@ -0,0 +1,10 @@
// RUN: %clang_cc1 -emit-llvm -triple powerpc64le-linux-gnu -verify %s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one RUN line (powerpc64) is sufficient. Duplicating this for ELF/XCOFF isn't necessary.

inline-asm-constraints-error.c might be a better test name. You can reuse the test for other errors.

@kamaub kamaub merged commit b8979c6 into llvm:main Jun 24, 2024
3 of 5 checks passed
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
'a' is an input/output constraint for restraining assembly variables
to an indexed or indirect address operand. It previously was marked
as supported but would throw an assertion for unknown constraint type
in the back-end when this test case was compiled. This change marks it
as unsupported until we can add full support for address operands
constraining to the compiler code generation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:PowerPC clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants