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

Eric Christopher echristo at gmail.com
Thu Dec 18 13:28:54 PST 2014


On Thu Dec 18 2014 at 1:21:53 PM Daniel Sanders <Daniel.Sanders at imgtec.com>
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.


>
> > 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.
>
>
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.

So, why would you put this patch in again?

-eric


>
> > (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 [echristo at gmail.com]
> *Sent:* 18 December 2014 20:29
> *To:* reviews+D6638+public+9dbc664d9cbf7b31 at reviews.llvm.org; Toma
> Tabacu; Daniel Sanders
> *Cc:* cfe-commits at cs.uiuc.edu
> *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
>
> On Thu Dec 18 2014 at 8:06:36 AM Toma Tabacu <toma.tabacu at imgtec.com>
> wrote:
>
>> Removed FIXME, as suggested in the review.
>>
>>
>> http://reviews.llvm.org/D6638
>>
>> Files:
>>   lib/Basic/Targets.cpp
>>   test/CodeGen/mips-constraint-regs.c
>>   test/CodeGen/mips-constraints-mem.c
>>   test/CodeGen/mips-inline-asm-modifiers.c
>>   test/CodeGen/mult-alt-generic.c
>>
>> Index: lib/Basic/Targets.cpp
>> ===================================================================
>> --- lib/Basic/Targets.cpp
>> +++ lib/Basic/Targets.cpp
>> @@ -5749,8 +5749,7 @@
>>    }
>>
>>    const char *getClobbers() const override {
>> -    // FIXME: Implement!
>> -    return "";
>> +    return "~{$1}";
>>    }
>>
>>    bool handleTargetFeatures(std::vector<std::string> &Features,
>> Index: test/CodeGen/mips-constraint-regs.c
>> ===================================================================
>> --- test/CodeGen/mips-constraint-regs.c
>> +++ test/CodeGen/mips-constraint-regs.c
>> @@ -9,7 +9,7 @@
>>    // '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__(
>> @@ -20,7 +20,7 @@
>>    // '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__(
>> @@ -32,7 +32,7 @@
>>    // '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;
>> Index: test/CodeGen/mips-constraints-mem.c
>> ===================================================================
>> --- test/CodeGen/mips-constraints-mem.c
>> +++ test/CodeGen/mips-constraints-mem.c
>> @@ -9,7 +9,7 @@
>>   // '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;
>>
>> Index: test/CodeGen/mips-inline-asm-modifiers.c
>> ===================================================================
>> --- test/CodeGen/mips-inline-asm-modifiers.c
>> +++ test/CodeGen/mips-inline-asm-modifiers.c
>> @@ -7,9 +7,9 @@
>>
>>  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()
>>  {
>> Index: test/CodeGen/mult-alt-generic.c
>> ===================================================================
>> --- test/CodeGen/mult-alt-generic.c
>> +++ test/CodeGen/mult-alt-generic.c
>> @@ -17,7 +17,7 @@
>>  // 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));
>>  }
>>
>> EMAIL PREFERENCES
>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141218/a2d61f65/attachment.html>


More information about the cfe-commits mailing list