[mips] Always clobber $1 for MIPS inline asm.

[tomatabacu]

Summary

Because GCC doesn't use $1 for code generation, inline assembly code can use $1 without having to add it to the clobbers list.

LLVM, on the other hand, does not shy away from using $1, and this can cause conflicts with inline assembly which assumes GCC-like code generation.

A solution to this problem is to make Clang automatically clobber $1 for all MIPS inline assembly.
This is not the optimal solution, but it seems like a necessary compromise, for now.

[dsanders]

LGTM

lib/Basic/Targets.cpp

5752

Not as far as I know. The user normally has to specify clobbers.

[tomatabacu]

Removed FIXME, as suggested in the review.

[echristo]

For the record, gcc does use $at for code generation, take a look at
mips_print_operand for the @ symbol and then look at all of the cases it's
being used.

This is a pretty weird hack, I think you could instead just look for uses
of $at or macro instructions (unless that other patch recently just set
nomacro for inline assembly).

-eric

[dsanders]

For the record, gcc does use $at for code generation, take a look at mips_print_operand for the @ symbol and then look at all of the cases it's being used.

That's correct. However, LLVM currently doesn't use $at for codegen and quite a bit of effort has been put in to making that the case. I'd like to retain this advantage over GCC if I can.

This is a pretty weird hack, I think you could instead just look for uses of $at or macro instructions

I think inspecting the assembly text to automatically discover clobbers would be a nice feature but it would mean that GCC wouldn't correctly handle inline assembly that works fine on LLVM. I don't think we need to do that now. The goal at the moment is to become compatible with GCC's inline assembly so that LLVM-compiled Linux works for Mips.

(unless that other patch recently just set nomacro for inline assembly).

The other patch effectively prepends '.set macro', '.set reorder', and '.set at' to the inline assembly to match the values that GCC is using for inline assembly (and codegen in general). After the inline assembly it reverts back to the mode LLVM normally uses. This is because there are quite a few inline assembly blocks in the linux kernel that expect .set macro/reorder/at.

---

From: Eric Christopher [[e-mail]]
Sent: 18 December 2014 20:29
To: [e-mail]; Toma Tabacu; Daniel Sanders
Cc: [e-mail]
Subject: Re: [PATCH] [mips] Always clobber $1 for MIPS inline asm.

For the record, gcc does use $at for code generation, take a look at mips_print_operand for the @ symbol and then look at all of the cases it's being used.

This is a pretty weird hack, I think you could instead just look for uses of $at or macro instructions (unless that other patch recently just set nomacro for inline assembly).

-eric

[dsanders]

On Thu Dec 18 2014 at 1:21:53 PM Daniel Sanders <[e-mail]<mailto:[e-mail]>> wrote:

For the record, gcc does use $at for code generation, take a look at mips_print_operand for the @ symbol and then look at all of the cases it's being used.

That's correct. However, LLVM currently doesn't use $at for codegen and quite a bit of effort has been put in to making that the case. I'd like to retain this advantage over GCC if I can.

That's not what the original mail said and there's no real reason to avoid using $at. I'm not sure I understand this rationale.

I see where we're confusing you, Toma and I aren't being precise enough. In GCC, $at is a reserved register. As you say, it does use $at in generated code but as far as I know, it only does so as a very short lived temporary. In LLVM, $at is an allocatable register and is freely used in all code generation.

In GCC inline assembly, $at is still a reserved register and users have taken advantage of this. In LLVM, $at needs to be clobbered by inline assembly so that the users usage of $at doesn't break the compiler generated code.

The point I was making is that the rationale for this patch is completely mistaken. i.e. gcc does use $at for code
generation so unless gcc is adding an implicit clobber of $at in inline asm then it's not going to be correct.

GCC goes further than an implicit clobber. It has promised not to keep any values in $at.

From: Eric Christopher [mailto:[e-mail]]
Sent: 18 December 2014 21:29
To: Daniel Sanders; [e-mail]; Toma Tabacu
Cc: [e-mail]
Subject: Re: [PATCH] [mips] Always clobber $1 for MIPS inline asm.

[echristo]

In D6638#103013, @dsanders wrote:

On Thu Dec 18 2014 at 1:21:53 PM Daniel Sanders <[e-mail]<mailto:[e-mail]>> wrote:

For the record, gcc does use $at for code generation, take a look at mips_print_operand for the @ symbol and then look at all of the cases it's being used.

That's correct. However, LLVM currently doesn't use $at for codegen and quite a bit of effort has been put in to making that the case. I'd like to retain this advantage over GCC if I can.

That's not what the original mail said and there's no real reason to avoid using $at. I'm not sure I understand this rationale.

I see where we're confusing you, Toma and I aren't being precise enough. In GCC, $at is a reserved register. As you say, it does use $at in generated code but as far as I know, it only does so as a very short lived temporary. In LLVM, $at is an allocatable register and is freely used in all code generation.

Yes...

In GCC inline assembly, $at is still a reserved register and users have taken advantage of this. In LLVM, $at needs to be clobbered by inline assembly so that the users usage of $at doesn't break the compiler generated code.

The point I was making is that the rationale for this patch is completely mistaken. i.e. gcc does use $at for code
generation so unless gcc is adding an implicit clobber of $at in inline asm then it's not going to be correct.

GCC goes further than an implicit clobber. It has promised not to keep any values in $at.

The idea behind the gcc usage is that anything that needs an explicit relocation or was a macro instruction before the demacroifying (yay, made up words) uses $at. We can definitely avoid using $at for anything in llvm, just pull it out of the register allocation pool?

I'm not sure where you're going with this?

-eric

[dsanders]

From: Eric Christopher [[e-mail]]
Sent: 06 January 2015 00:00
To: Toma Tabacu; Daniel Sanders
Cc: [e-mail]
Subject: Re: [PATCH] [mips] Always clobber $1 for MIPS inline asm.

In http://reviews.llvm.org/D6638#103013, @dsanders wrote:

On Thu Dec 18 2014 at 1:21:53 PM Daniel Sanders <[e-mail]<mailto:[e-mail]>> wrote:

> > For the record, gcc does use $at for code generation, take a look at mips_print_operand for the @ symbol and then look at all of the cases it's being used.
> That's correct. However, LLVM currently doesn't use $at for codegen and quite a bit of effort has been put in to making that the case. I'd like to retain this advantage over GCC if I can.

That's not what the original mail said and there's no real reason to avoid using $at. I'm not sure I understand this rationale.

I see where we're confusing you, Toma and I aren't being precise enough. In GCC, $at is a reserved register. As you say, it does use $a
in generated code but as far as I know, it only does so as a very short lived temporary. In LLVM, $at is an allocatable register and is
freely used in all code generation.

Yes...

In GCC inline assembly, $at is still a reserved register and users have taken advantage of this. In LLVM, $at needs to be clobbered by inline
assembly so that the users usage of $at doesn't break the compiler generated code.

The point I was making is that the rationale for this patch is completely mistaken. i.e. gcc does use $at for code
generation so unless gcc is adding an implicit clobber of $at in inline asm then it's not going to be correct.

GCC goes further than an implicit clobber. It has promised not to keep any values in $at.

The idea behind the gcc usage is that anything that needs an explicit relocation or was a macro instruction before
the demacroifying (yay, made up words) uses $at. We can definitely avoid using $at for anything in llvm, just pull it
out of the register allocation pool?

I'm not sure where you're going with this?

-eric

We could stop using $at but why is that beneficial? Why does universally reserving a register to support an (uncommon) construct make more sense to you than having that construct properly indicate that it may clobber a specific register? The former sounds like a sledgehammer to me.

The work pre-dates me but as far as I can tell the developers of the Mips port deliberately decided not to reserve $at and emit code in '.set noat' mode. Presumably this was to see how much of gcc's $at usage is essential to the correct operation of the output code. It turns out that the answer is 'almost none'. In LLVM's CodeGen, $at isn't particularly special. The few instructions/macros/relocations that need a temporary register to be $at describe this with an implicit def of AT. Branches like B are a good example of this (it's worth pointing out that most of the time we don't really need to clobber $at since the branch target will be in range). Where the temporary doesn't really need to be $at, we just ask the register allocator to give us any register. The hard work to avoid reserving $at is already done and I don't see a good reason to change the original decision.

[echristo]

We could stop using $at but why is that beneficial? Why does universally
reserving a register to support an (uncommon) construct make more sense to
you than having that construct properly indicate that it may clobber a
specific register? The former sounds like a sledgehammer to me.

Totally is, but blindly adding clobbers is much the same, just a more
focused hammer. I'm not against it, mostly that the original discussion
didn't say any of these things.

The work pre-dates me but as far as I can tell the developers of the Mips
port deliberately decided not to reserve $at and emit code in '.set noat'
mode. Presumably this was to see how much of gcc's $at usage is essential
to the correct operation of the output code. It turns out that

No, it's been known for a while that the use of macro instructions isn't a
good idea in the compiler. It's why we switched the gcc port away from
using them as well.

the answer is 'almost none'. In LLVM's CodeGen, $at isn't particularly
special. The few instructions/macros/relocations that need a temporary
register to be $at describe this with an implicit def of AT. Branches like
B are a good example of this (it's worth pointing out that most of the time
we don't really need to clobber $at since the branch target will be in
range). Where the temporary doesn't really need to be $at, we just ask the
register allocator to give us any register. The hard work to avoid
reserving $at is already done and I don't see a good reason to change the
original decision.

Agreed. Please do make sure that the places where you need to do this are
commented though. It's pretty poorly commented at the moment.

-eric

http://reviews.llvm.org/D6638

EMAIL PREFERENCES

http://reviews.llvm.org/settings/panel/emailpreferences/

[dsanders]

From: Eric Christopher [[e-mail]]
Sent: 06 January 2015 20:15
To: Toma Tabacu; Daniel Sanders
Cc: [e-mail]
Subject: Re: [PATCH] [mips] Always clobber $1 for MIPS inline asm.

We could stop using $at but why is that beneficial? Why does universally
reserving a register to support an (uncommon) construct make more sense to
you than having that construct properly indicate that it may clobber a
specific register? The former sounds like a sledgehammer to me.

Totally is, but blindly adding clobbers is much the same, just a more
focused hammer. I'm not against it, mostly that the original discussion
didn't say any of these things.

Agreed. It's currently a compromise between precision and difficulty of implementation.

The work pre-dates me but as far as I can tell the developers of the Mips
port deliberately decided not to reserve $at and emit code in '.set noat'
mode. Presumably this was to see how much of gcc's $at usage is essential
to the correct operation of the output code. It turns out that

No, it's been known for a while that the use of macro instructions isn't a
good idea in the compiler. It's why we switched the gcc port away from
using them as well.

Out of curiosity, when did the switch happen? I thought it was emitting macros last time I looked (probably around June) but I could be mistaken.

the answer is 'almost none'. In LLVM's CodeGen, $at isn't particularly
special. The few instructions/macros/relocations that need a temporary
register to be $at describe this with an implicit def of AT. Branches like
B are a good example of this (it's worth pointing out that most of the time
we don't really need to clobber $at since the branch target will be in
range). Where the temporary doesn't really need to be $at, we just ask the
register allocator to give us any register. The hard work to avoid
reserving $at is already done and I don't see a good reason to change the
original decision.

Agreed. Please do make sure that the places where you need to do this are
commented though. It's pretty poorly commented at the moment.

-eric

Toma: Could you add a suitable comment to getClobbers()?

http://reviews.llvm.org/D6638

[tomatabacu]

Commented the Clang code in r225632 and the LLVM code in r225521.

[dsanders]

No, it's been known for a while that the use of macro instructions isn't a
good idea in the compiler. It's why we switched the gcc port away from
using them as well.

Out of curiosity, when did the switch happen? I thought it was emitting macros last time I looked (probably around June) but I could be mistaken.

2001/2002? At one point it emitted things like la, etc.
-eric

In that case it seems much more likely that I misread the cases I'm remembering. Maybe the mnemonic made it look like a macro but the operands used meant it was really a plain instruction.

From: Eric Christopher [mailto:[e-mail]]
Sent: 07 January 2015 17:52
To: Daniel Sanders; [e-mail]; Toma Tabacu
Cc: [e-mail]
Subject: Re: [PATCH] [mips] Always clobber $1 for MIPS inline asm.

lib/Basic/Targets.cpp

case 'x': // hilo register pair
       return true;
     case 'R': // An address that can be used in a non-macro load or store
       Info.setAllowsMemory();
       return true;
     }
   }
 
   const char *getClobbers() const override {
-    // FIXME: Implement!
-    return "";
+    return "~{$1}";
   }
 
   bool handleTargetFeatures(std::vector<std::string> &Features,
                             DiagnosticsEngine &Diags) override {
     IsMips16 = false;
     IsMicromips = false;
     IsNan2008 = isNaN2008Default();
     IsSingleFloat = false;

test/CodeGen/mips-constraint-regs.c

 // RUN: %clang -target mipsel-unknown-linux -S -o - -emit-llvm %s \
 // RUN: | FileCheck %s
 
 // This checks that the frontend will accept inline asm constraints
 // c', 'l' and 'x'.
 
 int main()
 {
   // 'c': 16 bit address register for Mips16, GPR for all others
   // I am using 'c' to constrain both the target and one of the source
   // registers. We are looking for syntactical correctness.
-  // CHECK: %{{[0-9]+}} = call i32 asm sideeffect "addi $0,$1,$2 \0A\09\09", "=c,c,I"(i32 %{{[0-9]+}}, i32 %{{[0-9]+}}) [[NUW:#[0-9]+]], !srcloc !{{[0-9]+}}
+  // CHECK: %{{[0-9]+}} = call i32 asm sideeffect "addi $0,$1,$2 \0A\09\09", "=c,c,I,~{$1}"(i32 %{{[0-9]+}}, i32 %{{[0-9]+}}) [[NUW:#[0-9]+]], !srcloc !{{[0-9]+}}
   int __s, __v = 17;
   int __t;
   __asm__ __volatile__(
       "addi %0,%1,%2 \n\t\t"
       : "=c" (__t)
         : "c" (__s), "I" (__v));
 
   // 'l': lo register
   // We are making it clear that destination register is lo with the
   // use of the 'l' constraint ("=l").
-  // CHECK:   %{{[0-9]+}} = call i32 asm sideeffect "mtlo $1 \0A\09\09", "=l,r,~{lo}"(i32 %{{[0-9]+}}) [[NUW]], !srcloc !{{[0-9]+}}
+  // CHECK:   %{{[0-9]+}} = call i32 asm sideeffect "mtlo $1 \0A\09\09", "=l,r,~{lo},~{$1}"(i32 %{{[0-9]+}}) [[NUW]], !srcloc !{{[0-9]+}}
   int i_temp = 44;
   int i_result;
   __asm__ __volatile__(
       "mtlo %1 \n\t\t"
       : "=l" (i_result)
         : "r" (i_temp)
           : "lo");
 
   // 'x': Combined lo/hi registers
   // We are specifying that destination registers are the hi/lo pair with the
   // use of the 'x' constraint ("=x").
-  // CHECK:  %{{[0-9]+}} = call i64 asm sideeffect "mthi $1 \0A\09\09mtlo $2 \0A\09\09", "=x,r,r"(i32 %{{[0-9]+}}, i32 %{{[0-9]+}}) [[NUW]], !srcloc !{{[0-9]+}}
+  // CHECK:  %{{[0-9]+}} = call i64 asm sideeffect "mthi $1 \0A\09\09mtlo $2 \0A\09\09", "=x,r,r,~{$1}"(i32 %{{[0-9]+}}, i32 %{{[0-9]+}}) [[NUW]], !srcloc !{{[0-9]+}}
   int i_hi = 3;
   int i_lo = 2;
   long long ll_result = 0;
   __asm__ __volatile__(
       "mthi %1 \n\t\t"
       "mtlo %2 \n\t\t"
       : "=x" (ll_result)
         : "r" (i_hi), "r" (i_lo)
           : );
 
   return 0;
 }
 
 // CHECK: attributes [[NUW]] = { nounwind }

test/CodeGen/mips-constraints-mem.c

 // RUN: %clang -target mipsel-unknown-linux -S -o - -emit-llvm %s \
 // RUN: | FileCheck %s
 
 // This checks that the frontend will accept inline asm memory constraints.
 
 int foo()
 {
 
  // 'R': An address that can be used in a non-macro load or stor'
  // This test will result in the higher and lower nibbles being
  // switched due to the lwl/lwr instruction pairs.
- // CHECK:   %{{[0-9]+}} = call i32 asm sideeffect  "lwl $0, 1 + $1\0A\09lwr $0, 2 + $1\0A\09", "=r,*R"(i32* %{{[0-9,a-f]+}}) #1,
+ // CHECK:   %{{[0-9]+}} = call i32 asm sideeffect  "lwl $0, 1 + $1\0A\09lwr $0, 2 + $1\0A\09", "=r,*R,~{$1}"(i32* %{{[0-9,a-f]+}}) #1,
 
   int c = 0xffbbccdd;
 
   int *p = &c;
   int out = 0;
 
   __asm volatile (
     "lwl %0, 1 + %1\n\t"
     "lwr %0, 2 + %1\n\t"
     : "=r"(out)
     : "R"(*p)
     );
   return 0;
 }

test/CodeGen/mips-inline-asm-modifiers.c

 // RUN: %clang -target mipsel-unknown-linux -S -o - -emit-llvm %s \
 // RUN: | FileCheck %s
 
 // This checks that the frontend will accept inline asm operand modifiers
 
 int printf(const char*, ...);
 
 typedef int v4i32 __attribute__((vector_size(16)));
 
-  // CHECK: %{{[0-9]+}} = call i32 asm ".set noreorder;\0Alw    $0,$1;\0A.set reorder;\0A", "=r,*m"(i32* getelementptr inbounds ([8 x i32]* @b, i32 {{[0-9]+}}, i32 {{[0-9]+}})) #2,
-  // CHECK: %{{[0-9]+}} = call i32 asm "lw    $0,${1:D};\0A", "=r,*m"(i32* getelementptr inbounds ([8 x i32]* @b, i32 {{[0-9]+}}, i32 {{[0-9]+}})) #2,
-  // CHECK: %{{[0-9]+}} = call <4 x i32> asm "ldi.w ${0:w},1", "=f"
+  // CHECK: %{{[0-9]+}} = call i32 asm ".set noreorder;\0Alw    $0,$1;\0A.set reorder;\0A", "=r,*m,~{$1}"(i32* getelementptr inbounds ([8 x i32]* @b, i32 {{[0-9]+}}, i32 {{[0-9]+}})) #2,
+  // CHECK: %{{[0-9]+}} = call i32 asm "lw    $0,${1:D};\0A", "=r,*m,~{$1}"(i32* getelementptr inbounds ([8 x i32]* @b, i32 {{[0-9]+}}, i32 {{[0-9]+}})) #2,
+  // CHECK: %{{[0-9]+}} = call <4 x i32> asm "ldi.w ${0:w},1", "=f,~{$1}"
 int b[8] = {0,1,2,3,4,5,6,7};
 int  main()
 {
   int i;
   v4i32 v4i32_r;
 
   // The first word. Notice, no 'D'
   {asm (

test/CodeGen/mult-alt-generic.c

 
 int mout0;
 int min1;
 int marray[2];
 
 // CHECK: @single_m
 void single_m()
 {
-  // CHECK: call void asm "foo $1,$0", "=*m,*m[[CLOBBERS:[a-zA-Z0-9@%{},~_ ]*\"]](i32* {{[a-zA-Z0-9@%]+}}, i32* {{[a-zA-Z0-9@%]+}})
+  // CHECK: call void asm "foo $1,$0", "=*m,*m[[CLOBBERS:[a-zA-Z0-9@%{},~_$ ]*\"]](i32* {{[a-zA-Z0-9@%]+}}, i32* {{[a-zA-Z0-9@%]+}})
   asm("foo %1,%0" : "=m" (mout0) : "m" (min1));
 }
 
 // CHECK: @single_o
 void single_o()
 {
   register int out0 = 0;
   register int index = 1;