[PowerPC] Make "ca" aliased to "xer" #77557

bzEq · 2024-01-10T05:59:20Z

ca is not accepted in clobber list of inline assembly right now. Make ca aliased to xer, so it can be accepted in clobber list.

Fixes #77549.

llvmbot · 2024-01-10T05:59:51Z

@llvm/pr-subscribers-clang

Author: Kai Luo (bzEq)

Changes

ca is not accepted in clobber list of inline assembly right now. Make ca aliased to xer, so it can be accepted in clobber list.

Fixes #77549.

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

2 Files Affected:

(modified) clang/lib/Basic/Targets/PPC.cpp (+4-1)
(added) clang/test/CodeGen/ppc-register-names.c (+14)

diff --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 045c273f03c7a0..abf685f8883971 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -782,6 +782,9 @@ ArrayRef<const char *> PPCTargetInfo::getGCCRegNames() const {
 const TargetInfo::GCCRegAlias PPCTargetInfo::GCCRegAliases[] = {
     // While some of these aliases do map to different registers
     // they still share the same register name.
+    // Strictly speaking, "ca" is a subregister of "xer". However
+    // currently we don't model other bit fields of "xer", so treat
+    // "ca" aliasing to "xer".
     {{"0"}, "r0"},     {{"1", "sp"}, "r1"}, {{"2"}, "r2"},
     {{"3"}, "r3"},     {{"4"}, "r4"},       {{"5"}, "r5"},
     {{"6"}, "r6"},     {{"7"}, "r7"},       {{"8"}, "r8"},
@@ -803,7 +806,7 @@ const TargetInfo::GCCRegAlias PPCTargetInfo::GCCRegAliases[] = {
     {{"fr22"}, "f22"}, {{"fr23"}, "f23"},   {{"fr24"}, "f24"},
     {{"fr25"}, "f25"}, {{"fr26"}, "f26"},   {{"fr27"}, "f27"},
     {{"fr28"}, "f28"}, {{"fr29"}, "f29"},   {{"fr30"}, "f30"},
-    {{"fr31"}, "f31"}, {{"cc"}, "cr0"},
+    {{"fr31"}, "f31"}, {{"cc"}, "cr0"},     {{"ca"},   "xer"},
 };
 
 ArrayRef<TargetInfo::GCCRegAlias> PPCTargetInfo::getGCCRegAliases() const {
diff --git a/clang/test/CodeGen/ppc-register-names.c b/clang/test/CodeGen/ppc-register-names.c
new file mode 100644
index 00000000000000..209488c2e5f1ae
--- /dev/null
+++ b/clang/test/CodeGen/ppc-register-names.c
@@ -0,0 +1,14 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang -target powerpc64le -c %s -mllvm -stop-after=finalize-isel -o - | \
+// RUN:   FileCheck %s
+// RUN: %clang -target powerpc64 -c %s -mllvm -stop-after=finalize-isel -o - | \
+// RUN:   FileCheck %s
+
+void test_function(void) {
+  asm volatile("":::"ca");
+  asm volatile("":::"xer");
+  // CHECK: call void asm sideeffect "", "~{xer}"()
+  // CHECK: call void asm sideeffect "", "~{xer}"()
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $xer
+  // CHECK: INLINEASM &"", {{.*}} implicit-def early-clobber $xer
+}

github-actions · 2024-01-10T06:01:47Z

✅ With the latest revision this PR passed the C/C++ code formatter.

ecnelises · 2024-01-11T02:21:50Z

clang/test/CodeGen/ppc-register-names.c

+// RUN: %clang -target powerpc64le -c %s -mllvm -stop-after=finalize-isel -o - | \
+// RUN:   FileCheck %s
+// RUN: %clang -target powerpc64 -c %s -mllvm -stop-after=finalize-isel -o - | \
+// RUN:   FileCheck %s


Could we use %clang_cc1?

Looks %clang_cc doesn't accept -mllvm.

Why -c here? I think it will work with -S.

-mllvm -stop-after=finalize-isel should make -c and -S no difference, I think.

That's because clang -cc1 doesn't recognize -c, instead, it reads -emit-obj.

ecnelises · 2024-01-11T02:21:54Z

clang/lib/Basic/Targets/PPC.cpp

@@ -782,6 +782,8 @@ ArrayRef<const char *> PPCTargetInfo::getGCCRegNames() const {
 const TargetInfo::GCCRegAlias PPCTargetInfo::GCCRegAliases[] = {
    // While some of these aliases do map to different registers
    // they still share the same register name.
+    // Strictly speaking, "ca" is a subregister of "xer". However currently we


Should be a FIXME:?

Maybe a note. I don't think we are modeling other fields like SO, OV in XER in the near future.

The point of this patch was to allow the compiler to add "ca" as part of the clobber list.

However, even if the compiler doesn't track other bits in the XER we may have to add them so that the compiler doesn't complain when a user tries to specify the bit. If that happens, would we have to add those other fields and then specify them as proper subregs of XER?

Also, which of these bits, if any, does GCC support on PowerPC?

If that happens, would we have to add those other fields and then specify them as proper subregs of XER?

I think so. However, existing instructions look not using other fields like OV and OV32.

does GCC support on PowerPC?

Currently, GCC looks only support xer and ca. See https://godbolt.org/z/6qWqc3Pdd. I have tried using ov, hit CE.

bzEq · 2024-01-11T03:21:03Z

Add test for backend and adjust comment of CARRY.

stefanp-ibm · 2024-01-13T03:11:31Z

llvm/lib/Target/PowerPC/PPCRegisterInfo.td

-// Carry bit.  In the architecture this is really bit 0 of the XER register
-// (which really is SPR register 1);  this is the only bit interesting to a
-// compiler.
+// Carry bit. In the architecture this is really bit 2 of the 32-bit XER


nit:
The XER register is actually a 64 bit register. I realize that the first 32 bits are technically reserved (as well as a number of other bits like 35-43) but it is still a 64 bit register. In that way the carry bit becomes bit 34.

I also looked above and it seems that we incorrectly document SPR as being one of a number of 32 bit registers. The issue is that some of the SPR are 32 bit and others are 64 bit. It really depends on which SPR we are looking at.

I may be missing something here. Is there a history of the compiler assuming that all SPR are 32 bits?

Is there a history of the compiler assuming that all SPR are 32 bits?

The code was written more than a decade ago. I guess authors referenced The PowerPC Compiler Writer’s Guide. Figure 2-1 of the book shows XER, FPER, FPSCR are 32-bit width even under 64-bit environment.

In appendix, there is

Carry bit Bit 2 in the Fixed-Point Exception Register (XER).

I'll update it according to OpenPower's ISA, since it's more generally available.

stefanp-ibm · 2024-01-13T03:46:30Z

clang/lib/Basic/Targets/PPC.cpp

@@ -782,6 +782,8 @@ ArrayRef<const char *> PPCTargetInfo::getGCCRegNames() const {
 const TargetInfo::GCCRegAlias PPCTargetInfo::GCCRegAliases[] = {
    // While some of these aliases do map to different registers
    // they still share the same register name.
+    // Strictly speaking, "ca" is a subregister of "xer". However currently we


The point of this patch was to allow the compiler to add "ca" as part of the clobber list.

However, even if the compiler doesn't track other bits in the XER we may have to add them so that the compiler doesn't complain when a user tries to specify the bit. If that happens, would we have to add those other fields and then specify them as proper subregs of XER?

Also, which of these bits, if any, does GCC support on PowerPC?

bzEq added 2 commits January 10, 2024 05:29

Alias

e1caee4

CA aliasing to XER

aaee97f

bzEq requested review from chenzheng1030, orcguru, ecnelises, nemanjai, amy-kwan, EsmeYi, kamaub and stefanp-ibm January 10, 2024 05:59

bzEq self-assigned this Jan 10, 2024

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 10, 2024

Adjust comment

f871c97

ecnelises reviewed Jan 11, 2024

View reviewed changes

Backend stuff

9057159

Remove irrelevant change

5528307

stefanp-ibm reviewed Jan 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PowerPC] Make "ca" aliased to "xer" #77557

[PowerPC] Make "ca" aliased to "xer" #77557

bzEq commented Jan 10, 2024

llvmbot commented Jan 10, 2024

github-actions bot commented Jan 10, 2024 •

edited

ecnelises Jan 11, 2024

bzEq Jan 11, 2024

ecnelises Jan 11, 2024

bzEq Jan 11, 2024

ecnelises Jan 11, 2024

ecnelises Jan 11, 2024

bzEq Jan 11, 2024

stefanp-ibm Jan 13, 2024

bzEq Jan 16, 2024

bzEq commented Jan 11, 2024

stefanp-ibm Jan 13, 2024

bzEq Jan 16, 2024

bzEq Jan 16, 2024

stefanp-ibm Jan 13, 2024

[PowerPC] Make "ca" aliased to "xer" #77557

Are you sure you want to change the base?

[PowerPC] Make "ca" aliased to "xer" #77557

Conversation

bzEq commented Jan 10, 2024

llvmbot commented Jan 10, 2024

github-actions bot commented Jan 10, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bzEq commented Jan 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 10, 2024 •

edited