[PATCH] ms-inline-asm: Add facilities for rewriting labels in X86AsmParser

Ehsan Akhgari ehsan.akhgari at gmail.com
Fri Jul 18 13:08:25 PDT 2014


I'll address the rest of the comments in the next iteration of this and the LLVM side patch.

================
Comment at: lib/MC/MCParser/AsmParser.cpp:4620
@@ +4619,3 @@
+    case AOK_Identifier:
+      OS << AR.Identifier;
+      break;
----------------
Reid Kleckner wrote:
> This should really be making an local label that doesn't make it into the object file.  MC should know what the local label prefix is.  It's usually 'L', I think.
> 
> If you add the prefix here, then maybe this should be AOK_Label or something.
What do you mean by a local label?  I don't think that something that doesn't make it into the object file is what we want here.  For example, MSVC creates a relocation to $label$1 in this test case:

void f() {
  __asm {
    label:
    mov eax, label
  }
}

================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1305
@@ +1304,3 @@
+
+  // If the identifier lookup was unsuccessful, assume that we are dealing with
+  // a label.
----------------
Reid Kleckner wrote:
> What does MSVC do in this case?  I assume it uses the opposite logic for this code:
> 
>   void foo();
>   void __declspec(naked) bar() {
>     __asm {
>     foo:
>       jmp foo ; probably infinite loops instead of tail calling bar.
>     }
>   }
> 
> I guess this is the hard case:
> 
>   void foo();
>   void __declspec(naked) bar() {
>     __asm {
>       test ecx, ecx
>       xor eax, eax
>       jz foo
>       mov eax 1
>   foo:
>      ret
>   }
The first case is turned into:

_bar:
       0:       e9 00 00 00 00                                  jmp     0
                        00000001:  IMAGE_REL_I386_REL32 _foo

The second case is turned into:

_bar:
       0:       85 c9                                           testl   %ecx, %ecx
       2:       33 c0                                           xorl    %eax, %eax
       4:       0f 84 00 00 00 00                               je      0
                        00000006:  IMAGE_REL_I386_REL32 _foo
       a:       b8 01 00 00 00                                  movl    $1, %eax
       f:       c3                                              retl

My patch compiles the first case into:

_bar:
       0:	b8 00 00 00 00                               	movl	$0, %eax
			       1: IMAGE_REL_I386_DIR32	_foo

__MSASMLABEL_.0__:
       5:	ff e0                                        	jmpl	*%eax
       7:	c3                                           	ret

And the second case into:

_bar:
       0:	85 c9                                        	testl	%ecx, %ecx
       2:	31 c0                                        	xorl	%eax, %eax
       4:	0f 84 00 00 00 00                            	je	0
			       6: IMAGE_REL_I386_REL32	foo
       a:	b8 01 00 00 00                               	movl	$1, %eax

__MSASMLABEL_.0__:
       f:	c3                                           	ret
      10:	c3                                           	ret

Is that good (enough)?

http://reviews.llvm.org/D4587






More information about the llvm-commits mailing list