On Thu Dec 18 2014 at 1:21:53 PM Daniel Sanders <daniel.sand...@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 [echri...@gmail.com] > *Sent:* 18 December 2014 20:29 > *To:* reviews+d6638+public+9dbc664d9cbf7...@reviews.llvm.org; Toma > Tabacu; Daniel Sanders > *Cc:* cfe-commits@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.tab...@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@cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
