Skip to content

Conversation

@PiJoules
Copy link
Contributor

@PiJoules PiJoules commented Feb 18, 2025

https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#GenericOperandmodifiers provides the c and cc modifiers. GCC 15 introduces the cc modifier which does the same as c. This patch lets Clang handle this for compatibility.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 18, 2025

@llvm/pr-subscribers-clang

Author: None (PiJoules)

Changes

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

4 Files Affected:

  • (modified) clang/lib/AST/Stmt.cpp (+7)
  • (added) clang/test/AST/cc-modifier.cc (+10)
  • (added) clang/test/AST/cc-modifier.cpp (+10)
  • (modified) clang/test/CodeGen/asm.c (+6)
diff --git a/clang/lib/AST/Stmt.cpp b/clang/lib/AST/Stmt.cpp
index 685c00d0cb44f..c8ff2ecea20cc 100644
--- a/clang/lib/AST/Stmt.cpp
+++ b/clang/lib/AST/Stmt.cpp
@@ -708,6 +708,13 @@ unsigned GCCAsmStmt::AnalyzeAsmString(SmallVectorImpl<AsmStringPiece>&Pieces,
         DiagOffs = CurPtr-StrStart-1;
         return diag::err_asm_invalid_escape;
       }
+
+      // Specifically handle `cc` which we will alias to `c`.
+      // Note this is the only operand modifier that exists which has two
+      // characters.
+      if (EscapedChar == 'c' && *CurPtr == 'c')
+        CurPtr++;
+
       EscapedChar = *CurPtr++;
     }
 
diff --git a/clang/test/AST/cc-modifier.cc b/clang/test/AST/cc-modifier.cc
new file mode 100644
index 0000000000000..24eb105acd2b7
--- /dev/null
+++ b/clang/test/AST/cc-modifier.cc
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu  %s -verify
+// expected-no-diagnostics
+
+void func();
+void func2();
+
+bool func3() {                                                                                               
+  __asm__("%cc0 = %c1" : : "X"(func), "X"(func2));
+  return func2 == func;
+}                                                                                                                    
diff --git a/clang/test/AST/cc-modifier.cpp b/clang/test/AST/cc-modifier.cpp
new file mode 100644
index 0000000000000..24eb105acd2b7
--- /dev/null
+++ b/clang/test/AST/cc-modifier.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu  %s -verify
+// expected-no-diagnostics
+
+void func();
+void func2();
+
+bool func3() {                                                                                               
+  __asm__("%cc0 = %c1" : : "X"(func), "X"(func2));
+  return func2 == func;
+}                                                                                                                    
diff --git a/clang/test/CodeGen/asm.c b/clang/test/CodeGen/asm.c
index 10102cc2c4db1..9687c993e6464 100644
--- a/clang/test/CodeGen/asm.c
+++ b/clang/test/CodeGen/asm.c
@@ -284,3 +284,9 @@ void *t33(void *ptr)
   // CHECK: @t33
   // CHECK: %1 = call ptr asm "lea $1, $0", "=r,p,~{dirflag},~{fpsr},~{flags}"(ptr %0)
 }
+
+void t34(void) {
+  __asm__ volatile("T34 CC NAMED MODIFIER: %cc[input]" :: [input] "i" (4));
+  // CHECK: @t34()
+  // CHECK: T34 CC NAMED MODIFIER: ${0:c}
+}

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! The changes should come with a release note in clang/docs/ReleaseNotes.rst

I'm not super familiar with GCC Asm Statements, but the changes look correct to me otherwise.

https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#GenericOperandmodifiers
provides the `c` and `cc` modifiers. GCC 15 introduces the `cc` modifier
which does the same as `c`. This patch lets Clang handle this for
compatibility.
@PiJoules
Copy link
Contributor Author

PiJoules commented Feb 20, 2025

Thanks for the fix! The changes should come with a release note in clang/docs/ReleaseNotes.rst

I'm not super familiar with GCC Asm Statements, but the changes look correct to me otherwise.

Done

Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

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

It seems like for gcc at least, IIUC, cc does a bit more than c so while we are supporting cc for compatibility we are not fully supporting it? Specifically:

except try harder to print it with no punctuation

Perhaps we should document that in the comment in Stmt.cpp, for gcc comparability but not totally equivalent?

@petrhosek
Copy link
Member

It seems like for gcc at least, IIUC, cc does a bit more than c so while we are supporting cc for compatibility we are not fully supporting it? Specifically:

except try harder to print it with no punctuation

Perhaps we should document that in the comment in Stmt.cpp, for gcc comparability but not totally equivalent?

In our testing, Clang's c was already trying harder than GCC's c and in all the example we tried Clang's c matched GCC's cc. Do you know of any case where Clang's c is not equivalent to GCC's cc?

@shafik
Copy link
Collaborator

shafik commented Feb 27, 2025

It seems like for gcc at least, IIUC, cc does a bit more than c so while we are supporting cc for compatibility we are not fully supporting it? Specifically:
except try harder to print it with no punctuation
Perhaps we should document that in the comment in Stmt.cpp, for gcc comparability but not totally equivalent?

In our testing, Clang's c was already trying harder than GCC's c and in all the example we tried Clang's c matched GCC's cc. Do you know of any case where Clang's c is not equivalent to GCC's cc?

If that is the case then I think we are good.

@PiJoules PiJoules merged commit 70828d9 into llvm:main Feb 27, 2025
12 checks passed
@PiJoules PiJoules deleted the cc-modifier branch February 27, 2025 19:34
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 27, 2025

LLVM Buildbot has detected a new failure on builder sanitizer-x86_64-linux-android running on sanitizer-buildbot-android while building clang at step 2 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/6897

Here is the relevant piece of the build log for the reference
Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure)
...
[       OK ] AddressSanitizer.AtoiAndFriendsOOBTest (2215 ms)
[ RUN      ] AddressSanitizer.HasFeatureAddressSanitizerTest
[       OK ] AddressSanitizer.HasFeatureAddressSanitizerTest (0 ms)
[ RUN      ] AddressSanitizer.CallocReturnsZeroMem
[       OK ] AddressSanitizer.CallocReturnsZeroMem (14 ms)
[ DISABLED ] AddressSanitizer.DISABLED_TSDTest
[ RUN      ] AddressSanitizer.IgnoreTest
[       OK ] AddressSanitizer.IgnoreTest (0 ms)
[ RUN      ] AddressSanitizer.SignalTest
[       OK ] AddressSanitizer.SignalTest (193 ms)
[ RUN      ] AddressSanitizer.ReallocTest
[       OK ] AddressSanitizer.ReallocTest (37 ms)
[ RUN      ] AddressSanitizer.WrongFreeTest
[       OK ] AddressSanitizer.WrongFreeTest (128 ms)
[ RUN      ] AddressSanitizer.LongJmpTest
[       OK ] AddressSanitizer.LongJmpTest (0 ms)
[ RUN      ] AddressSanitizer.ThreadStackReuseTest
[       OK ] AddressSanitizer.ThreadStackReuseTest (1 ms)
[ DISABLED ] AddressSanitizer.DISABLED_MemIntrinsicUnalignedAccessTest
[ DISABLED ] AddressSanitizer.DISABLED_LargeFunctionSymbolizeTest
[ DISABLED ] AddressSanitizer.DISABLED_MallocFreeUnwindAndSymbolizeTest
[ RUN      ] AddressSanitizer.UseThenFreeThenUseTest
[       OK ] AddressSanitizer.UseThenFreeThenUseTest (120 ms)
[ RUN      ] AddressSanitizer.FileNameInGlobalReportTest
[       OK ] AddressSanitizer.FileNameInGlobalReportTest (126 ms)
[ DISABLED ] AddressSanitizer.DISABLED_StressStackReuseAndExceptionsTest
[ RUN      ] AddressSanitizer.MlockTest
[       OK ] AddressSanitizer.MlockTest (0 ms)
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadedTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowIn
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowLeft
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowRight
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFHigh
[ DISABLED ] AddressSanitizer.DISABLED_DemoOOM
[ DISABLED ] AddressSanitizer.DISABLED_DemoDoubleFreeTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoNullDerefTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoFunctionStaticTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoTooMuchMemoryTest
[ RUN      ] AddressSanitizer.LongDoubleNegativeTest
[       OK ] AddressSanitizer.LongDoubleNegativeTest (0 ms)
[----------] 19 tests from AddressSanitizer (27738 ms total)

[----------] Global test environment tear-down
[==========] 22 tests from 2 test suites ran. (27760 ms total)
[  PASSED  ] 22 tests.

  YOU HAVE 1 DISABLED TEST

Step 34 (run instrumented asan tests [aarch64/bluejay-userdebug/TQ3A.230805.001]) failure: run instrumented asan tests [aarch64/bluejay-userdebug/TQ3A.230805.001] (failure)
...
[ RUN      ] AddressSanitizer.HasFeatureAddressSanitizerTest
[       OK ] AddressSanitizer.HasFeatureAddressSanitizerTest (0 ms)
[ RUN      ] AddressSanitizer.CallocReturnsZeroMem
[       OK ] AddressSanitizer.CallocReturnsZeroMem (14 ms)
[ DISABLED ] AddressSanitizer.DISABLED_TSDTest
[ RUN      ] AddressSanitizer.IgnoreTest
[       OK ] AddressSanitizer.IgnoreTest (0 ms)
[ RUN      ] AddressSanitizer.SignalTest
[       OK ] AddressSanitizer.SignalTest (193 ms)
[ RUN      ] AddressSanitizer.ReallocTest
[       OK ] AddressSanitizer.ReallocTest (37 ms)
[ RUN      ] AddressSanitizer.WrongFreeTest
[       OK ] AddressSanitizer.WrongFreeTest (128 ms)
[ RUN      ] AddressSanitizer.LongJmpTest
[       OK ] AddressSanitizer.LongJmpTest (0 ms)
[ RUN      ] AddressSanitizer.ThreadStackReuseTest
[       OK ] AddressSanitizer.ThreadStackReuseTest (1 ms)
[ DISABLED ] AddressSanitizer.DISABLED_MemIntrinsicUnalignedAccessTest
[ DISABLED ] AddressSanitizer.DISABLED_LargeFunctionSymbolizeTest
[ DISABLED ] AddressSanitizer.DISABLED_MallocFreeUnwindAndSymbolizeTest
[ RUN      ] AddressSanitizer.UseThenFreeThenUseTest
[       OK ] AddressSanitizer.UseThenFreeThenUseTest (120 ms)
[ RUN      ] AddressSanitizer.FileNameInGlobalReportTest
[       OK ] AddressSanitizer.FileNameInGlobalReportTest (126 ms)
[ DISABLED ] AddressSanitizer.DISABLED_StressStackReuseAndExceptionsTest
[ RUN      ] AddressSanitizer.MlockTest
[       OK ] AddressSanitizer.MlockTest (0 ms)
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadedTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoThreadStackTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowIn
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowLeft
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFLowRight
[ DISABLED ] AddressSanitizer.DISABLED_DemoUAFHigh
[ DISABLED ] AddressSanitizer.DISABLED_DemoOOM
[ DISABLED ] AddressSanitizer.DISABLED_DemoDoubleFreeTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoNullDerefTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoFunctionStaticTest
[ DISABLED ] AddressSanitizer.DISABLED_DemoTooMuchMemoryTest
[ RUN      ] AddressSanitizer.LongDoubleNegativeTest
[       OK ] AddressSanitizer.LongDoubleNegativeTest (0 ms)
[----------] 19 tests from AddressSanitizer (27738 ms total)

[----------] Global test environment tear-down
[==========] 22 tests from 2 test suites ran. (27760 ms total)
[  PASSED  ] 22 tests.

  YOU HAVE 1 DISABLED TEST
program finished with exit code 0
elapsedTime=2306.320794

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

6 participants